[FLINK-40515][core] Extend BinaryVariant to support UUID - #29103
Open
manner wants to merge 1 commit into
Open
Conversation
Collaborator
raminqaf
reviewed
Sep 4, 2026
Comment on lines
+706
to
+707
| public static UUID getUUID(byte[] value, int pos) { | ||
| checkIndex(pos, value.length); |
Contributor
There was a problem hiding this comment.
Should we do a early check of value.length == 16 or is it always guarantied?
Contributor
Author
There was a problem hiding this comment.
Mhm, value.length should be the size of the whole variant data, not just the UUID, so I don't think we can compare to that size. And the bounds are checked inside readLongBigEndian() again if the byte[] is too short. Maybe you can clarify which check you mean.
Contributor
There was a problem hiding this comment.
value.length should be the size of the whole variant data
Answers my question!
raminqaf
reviewed
Sep 5, 2026
|
|
||
| UUID uuid = UUID.randomUUID(); | ||
| assertThat(builder.of(uuid).getUUID()).isEqualTo(uuid); | ||
| assertThat(builder.of(uuid).get()).isEqualTo(uuid); |
Contributor
There was a problem hiding this comment.
Do we have any test that evaluates broken values? Like an invalid UUID?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
Adds UUID (primitive type code 20) as a supported VARIANT primitive in
BinaryVariant.The 16-byte layout follows the open variant spec, which stores UUID big-endian. UUID is the only variant primitive that is not little-endian, so it uses dedicated big-endian read/write helpers.
Brief change log
BinaryVariantUtil: type mapping ingetType, acase UUIDinvalueSize, and thegetUUIDreaderwriteLongBigEndian/readLongBigEndian; a UUID is stored as its two 64-bit halves in 16 big-endian bytesappendUUIDtoBinaryVariantInternalBuilderandof(UUID)toBinaryVariantBuildergetUUID()andType.UUIDto theVariantinterface andof(UUID)toVariantBuilder, and wire UUID intoget()andtoJson()Verifying this change
This change added tests and can be verified as follows:
toJsonassertions for UUID variantstestUuidDecodeFromSpecBytes, which decodes the exact byte sequence from Iceberg'sTestSerializedPrimitives#testUUIDto confirm the wire format is compatible across implementations of the specDoes this pull request potentially affect one of the following parts:
@Public(Evolving): yes (VariantandVariantBuilderare@PublicEvolving, this adds new enum constants and new interface methods)Documentation
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Claude Opus 4.8)