LuciferYang opened a new issue, #9524:
URL: https://github.com/apache/paimon/issues/9524

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/paimon/issues) 
and found nothing similar.
   
   ### Paimon version
   
   master, `9c7deebbd` (2.1-SNAPSHOT)
   
   ### Compute Engine
   
   Java API (`paimon-common`). `BinaryRow` is `@Public`.
   
   ### Minimal reproduce step
   
   Every accessor in `BinaryRow` reads through the row's `offset`: `isNullAt` 
goes through `bitGet(segments[0], offset, ...)`, the field getters go through 
`getFieldOffset(pos)`, and even `getRowKind()` reads `segments[0].get(offset)`. 
`anyNull()` reads at absolute positions instead:
   
   ```java
   public boolean anyNull() {
       // Skip the header.
       if ((segments[0].getLong(0) & FIRST_BYTE_ZERO) != 0) {
           return true;
       }
       for (int i = 8; i < nullBitsSizeInBytes; i += 8) {
           if (segments[0].getLong(i) != 0) {
               return true;
           }
       }
       return false;
   }
   ```
   
   Point a row at a non-zero offset inside a segment that holds another row 
first, and `anyNull()` answers for the wrong row:
   
   ```java
   // segment holds rowWithoutNull followed by rowWithNull
   BinaryRow row = new BinaryRow(1);
   row.pointTo(segment, rowWithoutNullLength, rowWithNullLength);
   
   row.isNullAt(0);   // true, reads through the offset
   row.anyNull();     // false, read the leading row's null bits
   ```
   
   ### What doesn't meet your expectations?
   
   `anyNull()` and `isNullAt()` should never disagree about the same row. As 
written they agree only when the offset is zero.
   
   No Paimon code path hits this today: the four in-repo callers are in 
`FieldNestedUpdateAgg` and `FieldNestedPartialUpdateAgg`, and each passes a row 
from `keyProjection.apply(row).copy()`, where `BinaryRow.copy()` re-points the 
copy at offset 0. Non-zero offsets do occur elsewhere, from 
`BinaryRowSerializer.pointTo` when `mapFromPages` points a row at the bytes 
after a length prefix, so any future caller reaching `anyNull()` from that path 
would get a wrong answer with no error.
   
   ### Anything else?
   
   The other overload, `anyNull(int[] fields)`, goes through `isNullAt` and is 
correct. `BinaryArray.anyNull()` already reads from `offset + 4`, so this was 
the last place in that family reading at an absolute index.
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!
   


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