Repository navigation
Fix Row equality/hashing, RowUtils validation and ByteBuffer handling, and LogicalTypeCoder DATETIME conversion - #40789
Open
sjvanrossum wants to merge 10 commits into
Open
sjvanrossum wants to merge 10 commits into
sjvanrossum wants to merge 10 commits into
Conversation
… RowWithGetters equals/hashCode
…ls/hashCode contract
… SchemaCoderHelpers
Contributor
There was a problem hiding this comment.
📋 Review Summary
This PR fixes four distinct correctness issues across Row, RowWithGetters, RowUtils, and SchemaCoderHelpers.LogicalTypeCoder, ensuring proper equality/hashing contracts, nested Row schema validation, ByteBuffer slice/read-only conversion, and DATETIME logical type base conversions.
🔍 General Feedback
- The fixes are clean, minimal, and directly address the root causes of each bug.
- Unit test coverage across
RowTest,JavaFieldSchemaTest, andRowCoderTestthoroughly exercises all four fixes.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Contributor
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
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.
Fixes four correctness bugs across
Row,RowWithGetters,RowUtils, andSchemaCoderHelpers.LogicalTypeCoder.Row.Equals.deepHashCodepreviously passed the logical input value directly when recursing onfieldType.getLogicalType().getBaseType()without converting to the logical type's base value viaSchemaUtils.toLogicalBaseType(logicalType, a). When the base type wasBYTES(or a collection/map) and the input type was notbyte[](e.g.ByteBuffer),deepHashCodethrew aClassCastExceptioneven thoughRow.Equals.deepEqualsconverted to the base type first.RowWithGetters.equalsonly returnedtruewhen comparing twoRowWithGettersinstances ifObjects.equals(getterTarget, other.getterTarget)held, failing when the backing POJO/Bean did not overrideObject.equals, andRowWithGetters.hashCodehashed(getSchema(), getterTarget)instead of the row's field values, violating theObject.hashCodecontract whenever aRowWithGetterswas compared against an equivalentRowWithStorage.RowUtils.FieldValueBuilder.processRowpreviously returned an existingRowvalue without checking whether its schema matched the expected field schema, allowingRows with mismatched nested schemas (both inROWfields and inside collections/maps ofROW) to be constructed when field verification was bypassed or used viaFieldValueBuilder.RowUtilspreviously called((ByteBuffer) value).array()directly when convertingByteBuffervalues forBYTESfields. This threwUnsupportedOperationExceptionon read-only or directByteBuffers and silently ignoredposition(),limit(), andarrayOffset()on sliced/wrapped buffers, returning the entire backing array instead of the buffer's active slice.LogicalTypeCoder.encodeconvertedReadableInstantbase values toorg.joda.time.Instant(as required byInstantCoder), butstructuralValue,isRegisterByteSizeObserverCheap, andregisterByteSizeObservercalledlogicalType.toBaseType(value)directly without convertingReadableInstanttoInstant(and without guarding againstnull), causing aClassCastExceptioninInstantCoder.