comphead opened a new pull request, #6953: URL: https://github.com/apache/datafusion-comet/pull/6953
## Which issue does this PR close? There is no issue for this change. It continues the native Rust cleanup in #6861 and removes validity (null) masks that are rebuilt row by row. ## Rationale for this change Several native expressions rebuild a null mask that the input already carries. They append one bit per row to a `Vec<bool>` or a builder and pack it at the end, although the output is null exactly where an input is null. A `Vec<bool>` also uses a byte per row. `nested_extrema` collects a `Vec<Option<u32>>` only to convert it into an array afterwards. ## What changes are included in this PR? Reuse the input's nulls where the output is null exactly when the input is: - `from_json.rs`: the result struct is null only for a null input row, so `StructArray::try_new_with_length` takes `string_array.nulls().cloned()` instead of a `Vec<bool>` filled in the row loop. `try_new_with_length` is used because a struct with no fields needs the row count when there is no null buffer. The nested-struct builder has no input to borrow from, so its mask becomes a `BooleanBufferBuilder` instead of a `Vec<bool>`. - `regexp_extract_all.rs`: the result is null exactly where the subject is, so it reuses `array.nulls()`. - `split.rs`: `split_sql_generic_scalar_array` reuses the delimiter array's nulls and `split_sql_generic_array` uses `NullBuffer::union` of the string and delimiter nulls, instead of a `NullBufferBuilder` each. `split_generic` and `split_sql_generic_scalar` already worked this way. - `array_insert.rs`: a row is null when its list or its position is null, so `NullBuffer::union(list.nulls(), pos.nulls())` replaces the `Vec<bool>` that the row loop filled. Build an array without an intermediate `Vec`: - `array_extrema.rs`: `nested_extrema` appends to a `UInt32Builder` with `append_option`, as `float_extrema` in the same file does, instead of collecting `Vec<Option<u32>>` and calling `UInt32Array::from`. When the input has no nulls, the output now has no null buffer instead of an all-valid one. The two are logically equal. `arrays_zip.rs` has the same `Vec<bool>` pattern and is left alone on purpose. The function is a copy of DataFusion's `arrays_zip_inner` (the file says so and has a TODO to reuse it), so changing it here would drift from upstream. ## How are these changes tested? The results do not change, so existing tests cover them: `test_simple_struct`, `test_empty_and_null_json` (no nulls in the input) and `test_nested_struct` for `from_json`, `null_subject_returns_null` for `regexp_extract_all`, `test_split_sql_scalar_string_array_delimiter` for the scalar-string split, `test_array_insert`, and the SQL files `array_insert.sql` and `array_extrema_floating_point.sql` (arrays of arrays and of structs reach `nested_extrema`). The array-string by array-delimiter path of `split_sql` had no test, so this PR adds `test_split_sql_string_array_delimiter_array` with a NULL string and a NULL delimiter. `cargo clippy --all-targets --workspace -- -D warnings` and `cargo fmt --check` pass locally. The test suites were not run locally and there are no benchmark numbers. -- 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]
