Improve dictionary null handling in hashing and expand aggregate test coverage for nulls #16458

kosiew · 2025-06-19T14:09:06Z

Which issue does this PR close?

Closes Update Fuzz tests to include Dict with null values #16266

Rationale for this change

This change addresses a bug where combine_hashes was applied even if a dictionary value was null, leading to incorrect hash computations.
This was discovered while investigating #16266
Additionally, this PR extends the test coverage for aggregate functions to better validate behavior with dictionary arrays containing nulls.

What changes are included in this PR?

Fixes logic in hash_dictionary to ensure combine_hashes is only applied when the dictionary value is valid.
Corrects grammar in error messages for dataset generation expectations.
Enables null value generation in fuzz tests for dictionary arrays.
Adds comprehensive tests for aggregate functions (COUNT, SUM, MIN, MAX, MEDIAN, FIRST_VALUE, LAST_VALUE) using dictionary arrays with null keys and values.
Ensures consistent behavior across single and multi-partition execution.

Are these changes tested?

Yes, extensive new tests are added covering:

Aggregates on dictionary columns with null keys/values.
Window functions with null handling (IGNORE/RESPECT NULLS).
Partitioned vs. unpartitioned execution consistency.

Are there any user-facing changes?

No direct API changes, but query behavior involving dictionary arrays with nulls will now produce correct and consistent results in line with SQL semantics.

* Updated extending operators documentation * commented out Rust code to pass doc test --------- Co-authored-by: Andrew Lamb <[email protected]>

* feat(proto): udf decoding fallback * add test case for proto udf decode fallback

* Replace MSRV link on main page with Github badge

…CsvExec`, `JsonExec` (apache#16034)

Co-authored-by: Andrew Lamb <[email protected]>

Bumps [sqllogictest](https://github.com/risinglightdb/sqllogictest-rs) from 0.28.1 to 0.28.2. - [Release notes](https://github.com/risinglightdb/sqllogictest-rs/releases) - [Changelog](https://github.com/risinglightdb/sqllogictest-rs/blob/main/CHANGELOG.md) - [Commits](risinglightdb/sqllogictest-rs@v0.28.1...v0.28.2) --- updated-dependencies: - dependency-name: sqllogictest dependency-version: 0.28.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add lint rule to enforce string formatting style * format * extra * Update datafusion/ffi/src/tests/async_provider.rs Co-authored-by: kosiew <[email protected]> * Update datafusion/functions/src/datetime/to_date.rs Co-authored-by: kosiew <[email protected]> --------- Co-authored-by: kosiew <[email protected]>

…ache#16039) * Docs: Add example of creating a field in `return_field_from_args` * fmt * Update datafusion/expr/src/udf.rs Co-authored-by: Oleks V <[email protected]> * fmt --------- Co-authored-by: Oleks V <[email protected]>

* Fix comparisons between lists that contain nulls * Add support for lists in min/max agg functions * Add sqllogictests * Support lists in window frame target type

When aggregating first/last list over a column of lists, the first/last accumulators hold the necessary scalar value as is, which points to the list in the original input buffer. This results in two issues: 1) We prevent the deallocation of the input arrays which might be significantly larger than the single value we want to hold. 2) During aggreagtion with groups, many accumulators receive slices of the same input buffer, resulting in all held values pointing to this buffer. Then, when calculating the size of all accumulators we count the buffer multiple times, since each accumulator considers it to be part of its own allocation.

* Improve docs for Exprs and scalar functions * fix links

* h2o-window benchmark * Review: clarify h2o-window is an extended benchmark

Signed-off-by: Ruihang Xia <[email protected]>

* draft commit to rolledback changes on function naming and include prepare clause on the infer types tests * include data types in plan when it is not included in the prepare statement * fix: prepare statement error * Update datafusion/sql/src/statement.rs Co-authored-by: Andrew Lamb <[email protected]> * remove infer types from prepare statement the infer data type changes in statement will be introduced in a new PR * fix to show correct output message * remove white space * Restore the original tests too --------- Co-authored-by: Andrew Lamb <[email protected]>

…pache#16052)

* style: simplify some strings for readability * fix: formatting in `datafusion/` directory * refactor: replace long `format!` string * refactor: replace `format!` with `assert_eq!` --------- Co-authored-by: Andrew Lamb <[email protected]>

* support simple lateral joins Signed-off-by: Alex Chi Z <[email protected]> * fix explain test Signed-off-by: Alex Chi Z <[email protected]> * plan scalar agg correctly Signed-off-by: Alex Chi Z <[email protected]> * add uncorrelated query tests Signed-off-by: Alex Chi Z <[email protected]> * fix clippy + fmt Signed-off-by: Alex Chi Z <[email protected]> * make rule matching faster Signed-off-by: Alex Chi Z <[email protected]> * revert build_join visibility Signed-off-by: Alex Chi Z <[email protected]> * revert find plan outer column changes Signed-off-by: Alex Chi Z <[email protected]> * remove clone * address comment --------- Signed-off-by: Alex Chi Z <[email protected]> Co-authored-by: Alex Chi Z <[email protected]>

* chore(deps): bump the arrow-parquet group with 7 updates Bumps the arrow-parquet group with 7 updates: | Package | From | To | | --- | --- | --- | | [arrow](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | | [arrow-buffer](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | | [arrow-flight](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | | [arrow-ipc](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | | [arrow-ord](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | | [arrow-schema](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | | [parquet](https://github.com/apache/arrow-rs) | `55.0.0` | `55.1.0` | Updates `arrow` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) Updates `arrow-buffer` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) Updates `arrow-flight` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) Updates `arrow-ipc` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) Updates `arrow-ord` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) Updates `arrow-schema` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) Updates `parquet` from 55.0.0 to 55.1.0 - [Release notes](https://github.com/apache/arrow-rs/releases) - [Changelog](https://github.com/apache/arrow-rs/blob/main/CHANGELOG-old.md) - [Commits](apache/arrow-rs@55.0.0...55.1.0) --- updated-dependencies: - dependency-name: arrow dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet - dependency-name: arrow-buffer dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet - dependency-name: arrow-flight dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet - dependency-name: arrow-ipc dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet - dependency-name: arrow-ord dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet - dependency-name: arrow-schema dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet - dependency-name: parquet dependency-version: 55.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: arrow-parquet ... Signed-off-by: dependabot[bot] <[email protected]> * Update sqllogictest results --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andrew Lamb <[email protected]>

Bumps [petgraph](https://github.com/petgraph/petgraph) from 0.7.1 to 0.8.1. - [Release notes](https://github.com/petgraph/petgraph/releases) - [Changelog](https://github.com/petgraph/petgraph/blob/master/CHANGELOG.md) - [Commits](https://github.com/petgraph/petgraph/compare/[email protected]@v0.8.1) --- updated-dependencies: - dependency-name: petgraph dependency-version: 0.8.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add Spark-compatible char expression * Add slt test

* update window function * pretier fix * Update window_functions.md

Bumps [substrait](https://github.com/substrait-io/substrait-rs) from 0.55.1 to 0.56.0. - [Release notes](https://github.com/substrait-io/substrait-rs/releases) - [Changelog](https://github.com/substrait-io/substrait-rs/blob/main/CHANGELOG.md) - [Commits](substrait-io/substrait-rs@v0.55.1...v0.56.0) --- updated-dependencies: - dependency-name: substrait dependency-version: 0.56.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

`TempDir::into_path` "leaks" the temp dir. This updates the `tempfile` crate to a version where this method is deprecated and fixes all usages.

…tionary key hashes

…ry creation

…dict

…consistency

…meter

kosiew · 2025-06-20T04:03:19Z

Removing a large file from the commit history (test_data.txt) with git filter-repo messed up this branch.
Closing

kosiew · 2025-06-20T04:23:39Z

Replaced with #16466

the0ninjas and others added 30 commits May 12, 2025 13:48

Updated extending operators documentation (apache#15612)

d388670

* Updated extending operators documentation * commented out Rust code to pass doc test --------- Co-authored-by: Andrew Lamb <[email protected]>

feat(proto): udf decoding fallback (apache#15997)

10c685b

* feat(proto): udf decoding fallback * add test case for proto udf decode fallback

chore: Replace MSRV link on main page with Github badge (apache#16020)

075f4bb

* Replace MSRV link on main page with Github badge

Add note to upgrade guide for removal of ParquetExec, AvroExec, `…

a4e5afe

…CsvExec`, `JsonExec` (apache#16034)

refactor: remove deprecated ArrowExec (apache#16006)

7aa717c

refactor: remove deprecated MemoryExec (apache#16007)

946857e

refactor: remove deprecated JsonExec (apache#16005)

718408b

Co-authored-by: Andrew Lamb <[email protected]>

Use human-readable byte sizes in EXPLAIN (apache#16043)

f2e9c37

Support MIN and MAX for DataType::List (apache#16025)

db50b12

* Fix comparisons between lists that contain nulls * Add support for lists in min/max agg functions * Add sqllogictests * Support lists in window frame target type

Improve docs for Exprs and scalar functions (apache#16036)

916b266

* Improve docs for Exprs and scalar functions * fix links

Add h2o window benchmark (apache#16003)

673acab

* h2o-window benchmark * Review: clarify h2o-window is an extended benchmark

fix: track coalescer's consumption (apache#16048)

625c434

Signed-off-by: Ruihang Xia <[email protected]>

fix: Clarify that it is only the name of the field that is ignored (a…

7feaabd

…pache#16052)

Make error msg for oom human readable (apache#16050)

bc9ee4d

[datafusion-spark] Add Spark-compatible char expression (apache#15994)

37c802a

* Add Spark-compatible char expression * Add slt test

[Docs]: Added SQL example for all window functions (apache#16074)

2d7aeb4

* update window function * pretier fix * Update window_functions.md

Add test for collect_statistics (apache#16098)

da9412e

Add window function examples in code (apache#16102)

5560dfc

Refactor substrait producer into multiple files (apache#16089)

41e0433

Fix temp dir leak in tests (apache#16094)

bd76b64

`TempDir::into_path` "leaks" the temp dir. This updates the `tempfile` crate to a version where this method is deprecated and fixes all usages.

kosiew force-pushed the fuzz-16266a branch from 44bf0cf to b586808 Compare June 19, 2025 14:11

kosiew added 9 commits June 19, 2025 22:11

Merge branch 'main' into fuzz-16266a

fd2d6bb

fix: rename variables for clarity in hash_dictionary function

5ed1dfc

fix: refactor hash_dictionary to use helper function for updating dic…

0f6f050

…tionary key hashes

fix: add debug assertion for null percentage in RecordBatchGenerator

08fb46c

fix: introduce create_test_dict helper function for improved dictiona…

d25b703

…ry creation

refactor: remove legacy create_dict function in favor of create_test_…

00b1fe0

…dict

refactor: enhance test helper functions for improved readability and …

44b3f30

…consistency

feat: add fuzz timestamp test data structure and related setup functions

86db749

refactor: simplify run_snapshot_test function by removing unused para…

1968a37

…meter

kosiew force-pushed the fuzz-16266a branch from 2642e22 to 1968a37 Compare June 20, 2025 04:00

kosiew closed this Jun 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve dictionary null handling in hashing and expand aggregate test coverage for nulls #16458

Improve dictionary null handling in hashing and expand aggregate test coverage for nulls #16458

Uh oh!

kosiew commented Jun 19, 2025

Uh oh!

kosiew commented Jun 20, 2025 •

edited

Loading

Uh oh!

kosiew commented Jun 20, 2025

Uh oh!

Uh oh!

Improve dictionary null handling in hashing and expand aggregate test coverage for nulls #16458

Improve dictionary null handling in hashing and expand aggregate test coverage for nulls #16458

Uh oh!

Conversation

kosiew commented Jun 19, 2025

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

kosiew commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kosiew commented Jun 20, 2025

Uh oh!

Uh oh!

kosiew commented Jun 20, 2025 •

edited

Loading