GH-38990: [Java] Upgrade to flatc version 23.5.26#38991
Conversation
|
|
github-actions
Bot
added
Component: Java
Component: Documentation
awaiting review
| public static final String[] names = { "UNUSED", "DICTIONARY_REPLACEMENT", "COMPRESSED_BODY", }; | ||
|
|
||
| public static String name(int e) { return names[e]; } |
There was a problem hiding this comment.
This is the only change that looks like it could have downstream effects. Overall, I think it is benign if CI is successful.
There was a problem hiding this comment.
Hmm, weird that it doesn't generate this anymore. We should probably be conservative and note the breaking change, even if I don't think anyone would have directly touched the flatbufs.
There was a problem hiding this comment.
This could be a bug in the Java flatbuffers implementation. It seems an enum of type Long was updated (hence the 0L updates), so maybe it is forgetting to add names now. I'll check out the upstream source.
There was a problem hiding this comment.
I've also filed google/flatbuffers#8179 in the meantime.
There was a problem hiding this comment.
Okay, I verified that this is done by design for java flatbuffer generation (see issue above). Since Enum values actually reflect the long type when specified now, we can't create an array and try to index into it with a long value or we'll get a compiler error.
There was a problem hiding this comment.
Maybe we should make this Enum a short instead of a long here: https://github.com/apache/arrow/blob/main/format/Schema.fbs#L70
There was a problem hiding this comment.
I think we can't change that without breaking other things.
There was a problem hiding this comment.
That's fair. I wouldn't want to modify the format itself for this tbh.
github-actions
Bot
added
awaiting committer review
github-actions
Bot
added
awaiting merge
github-actions
Bot
added
awaiting changes
|
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit d555890. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 7 possible false positives for unstable benchmarks that are known to sometimes produce them. |
### Rationale for this change Upgrade flatc version to match the C++ implementation. ### What changes are included in this PR? * flatc upgraded to v23.5.26 and flatbuffer files regenerated ### Are these changes tested? Tested via CI/unit tests ### Are there any user-facing changes? Yes, but it's highly unlikely any user will use the flatbuffers directly. * Closes: apache#38990 Authored-by: Dane Pitkin <dane@voltrondata.com> Signed-off-by: David Li <li.davidm96@gmail.com>
### Rationale for this change Upgrade flatc version to match the C++ implementation. ### What changes are included in this PR? * flatc upgraded to v23.5.26 and flatbuffer files regenerated ### Are these changes tested? Tested via CI/unit tests ### Are there any user-facing changes? Yes, but it's highly unlikely any user will use the flatbuffers directly. * Closes: apache#38990 Authored-by: Dane Pitkin <dane@voltrondata.com> Signed-off-by: David Li <li.davidm96@gmail.com>
3 participants
Rationale for this change
Upgrade flatc version to match the C++ implementation.
What changes are included in this PR?
Are these changes tested?
Tested via CI/unit tests
Are there any user-facing changes?
Yes, but it's highly unlikely any user will use the flatbuffers directly.