sjvanrossum opened a new pull request, #40789: URL: https://github.com/apache/beam/pull/40789
Fixes four correctness bugs across `Row`, `RowWithGetters`, `RowUtils`, and `SchemaCoderHelpers.LogicalTypeCoder`. `Row.Equals.deepHashCode` previously passed the logical input value `a` directly when recursing on `fieldType.getLogicalType().getBaseType()` without converting `a` to the logical type's base value via `SchemaUtils.toLogicalBaseType(logicalType, a)`. When the base type was `BYTES` (or a collection/map) and the input type was not `byte[]` (e.g. `ByteBuffer`), `deepHashCode` threw a `ClassCastException` even though `Row.Equals.deepEquals` converted to the base type first. `RowWithGetters.equals` only returned `true` when comparing two `RowWithGetters` instances if `Objects.equals(getterTarget, other.getterTarget)` held, failing when the backing POJO/Bean did not override `Object.equals`, and `RowWithGetters.hashCode` hashed `(getSchema(), getterTarget)` instead of the row's field values, violating the `Object.hashCode` contract whenever a `RowWithGetters` was compared against an equivalent `RowWithStorage`. `RowWithGetters` now falls back to `super.equals(o)` when `getterTarget` equ ality does not fast-path, and delegates `hashCode()` to `super.hashCode()`. `RowUtils.FieldValueBuilder.processRow` previously returned an existing `Row` value without checking whether its schema matched the expected field schema, allowing `Row`s with mismatched nested schemas (both in `ROW` fields and inside collections/maps of `ROW`) to be constructed when field verification was bypassed or used via `FieldValueBuilder`. `processRow` now validates `schema.equals(retValue.getSchema())` and throws an `IllegalArgumentException` on mismatch. `RowUtils` previously called `((ByteBuffer) value).array()` directly when converting `ByteBuffer` values for `BYTES` fields. This threw `UnsupportedOperationException` on read-only or direct `ByteBuffer`s and silently ignored `position()`, `limit()`, and `arrayOffset()` on sliced/wrapped buffers, returning the entire backing array instead of the buffer's active slice. Added `RowUtils.toByteArray` to only return `buf.array()` when `buf.hasArray() && buf.arrayOffset() == 0 && buf.position() == 0 && buf.limit() == buf.capacity()`, and otherwise copy the `[position, limit)` range via `buf.duplicate().get(bytes)`. `LogicalTypeCoder.encode` converted `ReadableInstant` base values to `org.joda.time.Instant` (as required by `InstantCoder`), but `structuralValue`, `isRegisterByteSizeObserverCheap`, and `registerByteSizeObserver` called `logicalType.toBaseType(value)` directly without converting `ReadableInstant` to `Instant` (and without guarding against `null`), causing `ClassCastException: org.joda.time.DateTime cannot be cast to org.joda.time.Instant` in `InstantCoder`. Extracted a shared `toBaseType(InputT)` helper in `LogicalTypeCoder` that handles `null` and `ReadableInstant.toInstant()` consistently across all coder methods. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
