LuciferYang opened a new pull request, #9525:
URL: https://github.com/apache/paimon/pull/9525

   ### Purpose
   
   close #9524
   
   `BinaryRow.anyNull()` read the header word and the null-bit words from 
`segments[0]` at absolute positions 0 and `i`, while every other accessor in 
the class reads through the row's `offset`: `isNullAt` goes through 
`bitGet(segments[0], offset, ...)`, the field getters through 
`getFieldOffset(pos)`, and `getRowKind()` reads `segments[0].get(offset)`. A 
row pointed at a non-zero offset therefore answered from whatever bytes sat at 
the start of the segment, so `anyNull()` and `isNullAt()` could disagree about 
the same row. Both reads now add the offset.
   
   The four in-repo callers, in `FieldNestedUpdateAgg` and 
`FieldNestedPartialUpdateAgg`, each pass a row from 
`keyProjection.apply(row).copy()`, and `BinaryRow.copy()` re-points the copy at 
offset 0, so no current code path is affected. Non-zero offsets do occur: 
`BinaryRowSerializer.pointTo` gives a row the position after a length prefix 
when `mapFromPages` walks a page, so the first row in a page sits at offset 4.
   
   `anyNull(int[] fields)` needed no change; it goes through `isNullAt`. 
`BinaryArray.anyNull()` already reads from `offset + 4`, so this was the last 
member of that family reading at an absolute index.
   
   ### Tests
   
   Two cases next to the existing `anyNullTest` in `BinaryRowTest`, one per 
code path in the method, both laying two rows into one segment with a shared 
`concat` helper and pointing the second one at a non-zero offset.
   
   - `testAnyNullWithNonZeroOffset`: arity 1, so only the header read runs. The 
row starts at offset 20, four bytes of padding plus a 16-byte row, so the 
offset is not a multiple of eight either, which is the alignment 
`BinaryRowSerializer` actually produces.
   - `testAnyNullHighFieldWithNonZeroOffset`: arity 60 with the null at field 
59, which puts the null bit in the second 8-byte word, so the loop is what has 
to honor the offset. It also asserts `isNullAt(59)` first, so a failure cannot 
be blamed on the fixture.
   
   In both cases the row without nulls goes first, so a read that ignores the 
offset lands on it and reports no null. Both fail against the pre-fix code.
   
   `mvn -pl paimon-common test` on JDK 8: 12467 tests, 0 failures, 0 errors. 
checkstyle, spotless, enforcer and rat run clean.
   


-- 
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]

Reply via email to