API: Compute variant element sizes in VariantTestUtil - #17934
Open
dkranchii wants to merge 1 commit into
Open
Conversation
Replace the direct `VariantValue.sizeInBytes()` calls used by `VariantTestUtil.createArray` and `createObject` with a shared `sizeInBytes(VariantValue)` helper that derives the serialized byte size from the physical type (recursing into arrays and returning `buffer().remaining()` for pre-serialized values). This resolves the existing TODO in `createArray` and applies the same treatment to `createObject`, making the test util robust when nested or non-buffer-backed values are supplied.
dkranchii
force-pushed
the
variant-test-util-compute-sizes
branch
from
September 3, 2026 00:30
195bca6 to
6b29651
Compare
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
1 participant
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.
Summary
VariantTestUtil.createArrayby introducing a sharedsizeInBytes(VariantValue)helper that derives serialized byte size from the physical type instead of callingVariantValue.sizeInBytes()directly.VariantTestUtil.createObject, so both container builders compute sizes consistently and remain correct for nested or non-buffer-backed values.buffer().remaining()for pre-serialized values, computes primitives from theirPhysicalType(with special handling forSTRING/BINARY), recurses into arrays, and delegates toVariantValue.sizeInBytes()only for non-serializedOBJECT(whose field-id size requires the object's own metadata).Closes #17510.
Test plan
TestVariantTestUtilcovers the helper for serialized primitives, short strings, long strings, flat and nested serialized arrays, and asserts that the computed size equals the number of bytes actually written bywriteTo.TestSerializedArrayandTestSerializedObjectcontinue to exercisecreateArray/createObjectend-to-end../gradlew spotlessCheck./gradlew :iceberg-api:testAI Disclosure