morningman commented on PR #69581:
URL: https://github.com/apache/doris/pull/69581#issuecomment-6653888452

   Re: [P1] Avoid rebuilding the entire child validity bitmap for every Arrow 
output block 
(https://github.com/apache/doris/pull/69581#issuecomment-6621254086)
   
   Addressed in `42cf4e3e5d1`, on top of the rebase onto master `315c2e00331`.
   
   - `mask_arrow_children_hidden_by_null_parents()` now reads a NULL row's 
child span before it allocates. A span that is empty, or whose slots are NULL 
already, hides nothing, so such a row allocates no bitmap, and the child itself 
is returned when no NULL row in the range hides a valid value. This is what 
`mask_unreferenced_slots()` on the Lance path already did; the two now share 
`has_valid_slot()`. The alternating `[1]` / NULL lists in your example cost no 
bitmap at all.
   - `ArrowStreamReader::_prepare_arrays()` masks each retained IPC batch once 
with `mask_arrow_descendants_not_visible()`, using the reader's accounted pool, 
before `get_block()` hands its row ranges to the SerDes. A large LIST, 
FIXED_SIZE_LIST or STRUCT column whose NULL rows hide valid values is therefore 
masked once per batch, and the per-block SerDe calls find nothing left to hide.
   - What remains per call: map keys under a NULL map row that still owns 
entries, because a MapArray cannot hold NULL keys (Arrow's map builder writes 
NULL map rows without entries). paimon-rust also reads row ranges of one batch, 
but its batches are about the size of a block.
   
   Tests: `ArrowParentNullsTest.AllocatesNothingForNullRowsThatHideNothing` 
(empty and already-NULL spans allocate nothing from the Doris Arrow pool; a row 
that hides a valid value is still masked) and 
`LoadArrowPipeTest.Null{List,FixedSizeList,Map,Struct}BatchReadsInBlocksWithoutPerBlockMasks`
 (one 64-row IPC batch read four rows per block: no block allocates from the 
masking pool, and every row reads as a whole-batch read does). Turning either 
change off fails its cases: allocating at the first NULL row of each call again 
fails all five, and dropping the per-batch mask fails the LIST and STRUCT block 
cases. `DataTypeDateTimeV2SerDeTest.IgnoresArrowTimestampsHiddenByNullParents` 
adds the NULL-parent case of the #69697 range checks: an out-of-range 
DATETIMEV2 / TIMESTAMPTZ under a NULL LIST, FIXED_SIZE_LIST, MAP value or 
STRUCT row reads as NULL, while the same value in a visible row still fails.
   


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to