morningman commented on PR #69581: URL: https://github.com/apache/doris/pull/69581#issuecomment-6461955158
Addressed the sixth review round in `1c74dbab92c`, and in `d320776955b` the same problem in the compaction path, which the review did not raise. Per finding: - 4395648413 (P2, Decimal256 payloads hidden by NULL parents): fixed in `1c74dbab92c`. Before a nested Decimal256 leaf is narrowed, every child slot that no visible row references is marked NULL, level by level, so only values a reader can see are cast; a visible value outside the declared precision is still rejected. Map keys cannot be NULL and stay checked. - 4395648419 (P2, mask bitmaps outside the query memory tracker): fixed in `1c74dbab92c`; the bitmaps now come from ExecEnv's tracked Arrow pool. The scale is smaller than described, see the thread. - Compaction (not raised by the review): While fixing the hidden Decimal256 values, we found the same problem one step later, in compaction. A nested Lance array that needs a compacted copy (a sliced batch, or a level whose offsets or lengths do not line up with its child) goes through Arrow's `ValidateFull()` before it is copied, and full validation checks every non-NULL value of each descendant over the descendant's whole length: UTF-8 for strings, the precision of decimals, whole days for Date64 and the day range for Time32/Time64. That includes the slots under a NULL list, fixed-size-list, map or struct row and those of rows outside the slice, so a batch failed with `validate sliced Lance array failed` on a value that no row shows, for example invalid UTF-8 under a NULL fixed-size-list row or an out-of-precision decimal256(40, 2) value under a NULL list row. In `d320776955b`, `compact_lance_array()` first marks NULL every slot that no visible row reaches, at every nesting leve l, with the same mask the normalizer applies before narrowing; map keys stay unmasked, because Arrow does not allow a NULL key. `ValidateFull()` still runs on the masked array, so a visible invalid value is still rejected and its offset and length checks still guard the copy. The copied rows are unchanged, slots that are NULL already, such as those the narrowing masked, get no second bitmap, nothing is allocated when no slot is hidden, and the bitmaps come from the converter's tracked Arrow pool. Covered by `LanceTableReaderTypeTest.CompactsArraysWithInvalidPayloadsHiddenByNullParents`, `LanceTableReaderTypeTest.RejectsVisibleInvalidPayloadsInCompactedArrays`, `ArrowParentNullsTest.MasksHiddenSlotsAtEveryNestingLevel` and `ArrowParentNullsTest.LeavesVisibleAndAlreadyMaskedArraysUnchanged`. BE unit tests on macOS arm64 (ASAN): `ArrowParentNullsTest`, the Arrow SerDe suites, the Lance and ADBC suites and `IcebergV2ReaderTest`, 310 pass, 7 ADBC SQLite cases skip (no local SQLite driver). Each new test fails under a mutation of the code it guards. -- 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]
