Add warning about using Schema.FieldType.BYTES keys in a Map #31600
Conversation
Amar3tto
force-pushed
the
bytes-keys-in-map
branch
from
d4c9e31 to
478e7bc
Compare
|
Run Java PreCommit |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
R: @Abacn |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
| @@ -816,6 +821,13 @@ public static FieldType iterable(FieldType elementType) { | |||
|
|
|||
| /** Create a map type for the given key and value types. */ | |||
| public static FieldType map(FieldType keyType, FieldType valueType) { | |||
| if (FieldType.BYTES.equals(keyType)) { | |||
There was a problem hiding this comment.
This is a valid warning, however may cause log spam as the code path is visited on every value.
A Possible solution is to have a private static flag to track if the warning has been emitted so emit this error once.
There was a problem hiding this comment.
I am not sure that it will cause spam, just one message per one creating of FieldType with byte array key in a map.
It's not clear where to have this flag, because FieldType object may exist independently of Schema, in such case we want to have a warning too. What do you think?
There was a problem hiding this comment.
yeah you are right, here is create FieldType instead of FieldValue
Resolves #26695
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.