Core: Fix ShreddedObject.put not clearing prior remove marker#17066
Open
thswlsqls wants to merge 1 commit into
Open
Core: Fix ShreddedObject.put not clearing prior remove marker#17066thswlsqls wants to merge 1 commit into
thswlsqls wants to merge 1 commit into
Conversation
put(field, value) added the field back to shreddedFields but left any prior removedFields marker for that field in place. get(), fieldNames(), and numFields() short-circuit on removedFields, so a remove() followed by a put() of the same field made query APIs report the field as missing while writeTo() still serialized it with its new value. Clear the removedFields marker in put(), mirroring how remove() already clears the shreddedFields entry, so query results and serialized bytes agree. Adds a regression test in TestShreddedObject covering the remove-then-put sequence via get()/numFields()/fieldNames() and a writeTo() round trip. Generated-by: Claude Code
nssalian
approved these changes
Jul 5, 2026
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.
Closes #17063
Summary
ShreddedObject.put(field, value)left a priorremove(field)marker inremovedFields, soremove()followed byput()of the same field madeget()/fieldNames()/numFields()report the field missing whilewriteTo()still serialized it with the new value.removedFields.remove(field)input(), mirroring howremove()already clearsshreddedFieldsfor the field.Testing done
TestShreddedObject#testPutAfterRemoveClearsRemoveMarkercoveringremove()thenput()of the same field, assertingget()/numFields()/fieldNames()reflect the new value and thewriteTo()round trip serializes it../gradlew :iceberg-core:test --tests "org.apache.iceberg.variants.TestShreddedObject"— 23 tests passed../gradlew :iceberg-core:check— spotlessCheck, checkstyleMain/Test/Jmh passed; full test suite timed out in CI session (large module), covered by the targeted test above instead../gradlew :iceberg-core:revapi— passed (backward compatible, no signature change).