[AURON #2419] RowNullChecker may mis-detect null join keys for complex Arrow row types#2424
[AURON #2419] RowNullChecker may mis-detect null join keys for complex Arrow row types#2424weimingdiit wants to merge 1 commit into
Conversation
…complex Arrow row types Signed-off-by: weimingdiit <weimingdiit@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect join-key null detection for complex Arrow row encodings (e.g., struct/list/dictionary) by switching RowNullChecker::has_nulls from manual byte inspection to decoding rows via Arrow’s RowConverter, then deriving a null-key mask from the decoded top-level key arrays.
Changes:
- Add and retain an Arrow
RowConverterinsideRowNullCheckerfor row decoding. - Rework
has_nullsto decodeRowsinto key columns and compute validity based on top-level nulls. - Add unit tests covering complex key types (struct, list, dictionary) and ensuring nested nulls don’t invalidate a non-null top-level key.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let parser = self.row_converter.parser(); | ||
| let parsed_rows = rows | ||
| .iter() | ||
| .map(|row| parser.parse(row.data())) | ||
| .collect::<Vec<_>>(); | ||
| let key_columns = self | ||
| .row_converter | ||
| .convert_rows(parsed_rows) | ||
| .expect("failed to decode row data in RowNullChecker"); | ||
|
|
||
| NullBuffer::from_iter( | ||
| (0..rows.num_rows()) | ||
| .map(|row_index| key_columns.iter().all(|column| !column.is_null(row_index))), | ||
| ) |
| let parser = self.row_converter.parser(); | ||
| let parsed_rows = rows | ||
| .iter() | ||
| .map(|row| parser.parse(row.data())) | ||
| .collect::<Vec<_>>(); | ||
| let key_columns = self | ||
| .row_converter | ||
| .convert_rows(parsed_rows) | ||
| .expect("failed to decode row data in RowNullChecker"); | ||
|
|
||
| NullBuffer::from_iter( | ||
| (0..rows.num_rows()) | ||
| .map(|row_index| key_columns.iter().all(|column| !column.is_null(row_index))), |
There was a problem hiding this comment.
Thanks for fixing null detection for complex row types. However, has_nulls is called for every sort-merge join batch, and this change now fully decodes all key columns even for primitive keys. Could we preserve the existing fast path for simple types and only decode complex types to avoid a regression in the common join path?
Which issue does this PR close?
Closes #2419
Rationale for this change
Auron's sort merge join uses encoded Arrow rows for join key comparison and uses RowNullChecker to skip rows with null join keys.
RowNullChecker previously inspected Arrow row bytes manually. This is fragile for complex row encodings such as struct, list, and dictionary, because their layout is controlled by Arrow RowConverter and should not be interpreted with fixed or estimated field lengths.
This can cause RowNullChecker to mis-detect null join keys for complex key types.
What changes are included in this PR?
Adds an Arrow RowConverter to RowNullChecker.
Updates RowNullChecker's
has_nullspath to decode Rows through Arrow RowConverter before building the null-key mask.Preserves conservative join key null semantics:
top-level null key values are treated as null join keys
nested null fields or elements inside non-null struct/list keys do not make the key null
Keeps the existing RowNullChecker public method signatures unchanged.
Keeps the existing byte-level helper path unchanged.
Adds unit coverage for complex struct, list, and dictionary key rows.
Are there any user-facing changes?
No user-facing API changes.
How was this patch tested?
UT.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-5)