Skip to content

feat(spark): add equal_null - #24827

Merged
kosiew merged 5 commits into
apache:mainfrom
advitrocks9:b8-spark-equal-null
Sep 22, 2026
Merged

kosiew merged 5 commits into
apache:mainfrom
advitrocks9:b8-spark-equal-null

Conversation

@advitrocks9

@advitrocks9 advitrocks9 commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

No issue filed. Part of apache/datafusion-spark#5, which tracks the Spark function library.

Rationale for this change

equal_null is missing, and the misc module it belongs in had no functions in it at all. In
Spark it is an alias: EqualNull(l, r) is rewritten to EqualNullSafe, the <=> operator, so
it is null-safe equality. Both NULL is true, and the result is never NULL:

SELECT equal_null(NULL, NULL);              -- true
SELECT equal_null(NULL::int, 1::int);       -- false

DataFusion already has that operator as IS NOT DISTINCT FROM, so this mirrors Spark's own
structure instead of writing a second comparison kernel.

What changes are included in this PR?

misc/equal_null.rs and its registration. simplify() rewrites to the operator, which is
Spark's replacement field, and invoke_with_args calls apply_cmp with the same operator so
the function also works with the logical optimizer disabled, as the crate README requires for
Comet.

Two of the stub's commented-out queries were malformed, the porting script wrote one cast per
distinct typeof() key, so a call with two identical literals lost an argument. The same
artifact affects 10 more pairs in 6 other spark files, left alone here.

Two divergences from Spark are left alone because they belong to the operator, not to this
function. Spark treats -NaN and NaN as equal and DataFusion does not, which is true of
IS NOT DISTINCT FROM generally. Spark also rejects maps at analysis since MapType is not
orderable, while comparison_coercion here accepts them.

return_field_from_args is overridden so the field is not nullable. normalize_float_zero now
recurses into nested children. compare_op_for_nested and GroupValuesColumn both normalize
through it, so the scalar kernels and the two group-by paths agree on nested float keys.

Are these changes tested?

Yes, spark/misc/equal_null.slt goes from a skipped stub to 27 assertions covering the truth
table, float ordering, columns, arrays, structs, decimals and the arity errors.

Reverting simplify() to plain Eq fails 9 of them, so the file is not passing on constant
folding. Putting invoke_with_args back to a stub fails the two queries that run with the
optimizer off, and nothing else.

Are there any user-facing changes?

Yes. equal_null is new in datafusion-spark, and -0.0 now matches +0.0 inside nested
values, in comparisons and in GROUP BY, matching the scalar kernels.

Rewrites to IS NOT DISTINCT FROM in simplify(), mirroring Spark's own
RuntimeReplaceable alias of EqualNullSafe. Fills the porting-script stub,
restoring the two queries that lost an argument to the typeof() dedup.
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) spark Changes to datafusion-spark crate labels Aug 31, 2026
@codecov-commenter

codecov-commenter commented Aug 31, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.32258% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.36%. Comparing base (edc936f) to head (2ef1d12).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/spark/src/function/misc/equal_null.rs 79.54% 7 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   apache/datafusion#24827      +/-   ##
==========================================
+ Coverage   82.33%   82.36%   +0.02%     
==========================================
  Files        1137     1138       +1     
  Lines      432498   433029     +531     
  Branches   432498   433029     +531     
==========================================
+ Hits       356116   356654     +538     
  Misses      54843    54843              
+ Partials    21539    21532       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@advitrocks9,

Thanks for working on adding equal_null support. The overall approach looks good, especially reusing IS NOT DISTINCT FROM rather than introducing a separate comparison implementation.

I found two correctness issues that I think should be addressed before merging. One is around the nullability reported by the UDF when simplification is disabled, and the other is an inconsistency in signed-zero handling for nested values. I left inline comments with reproduction cases and suggested fixes.

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates common Related to common crate labels Sep 18, 2026
@advitrocks9

Copy link
Copy Markdown
Contributor Author

Both are fixed.

For nullability I overrode return_field_from_args since the default marks the field nullable. I also pinned the schema with datafusion.explain.show_schema instead of EXPLAIN VERBOSE so the optimizer pass list does not get copied into the file.

For signed zero this was not specific to equal_null. [0.0] = [-0.0] was also false on main. I fixed it in compare_op_for_nested by making normalize_float_zero recurse into nested children.

@advitrocks9
advitrocks9 requested a review from kosiew September 18, 2026 00:07
@kosiew

kosiew commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

@advitrocks9
Can you resolve the CI errors?

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Sep 18, 2026
@advitrocks9

advitrocks9 commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor Author

@advitrocks9 Can you resolve the CI errors?

@kosiew All green now

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@advitrocks9, thanks for the updates here. This looks good to me.

The nullability concern is addressed by return_field_from_args returning a non-nullable Boolean field, and the optimizer-disabled EXPLAIN test confirms that behavior.

The nested signed-zero handling also looks good. Normalizing at the shared nested-comparison boundary covers both regular nested comparisons and the equal_null fallback path, with coverage for scalar and optimizer-disabled array cases.

I also took another look at the grouping coverage. My earlier suggestion to add a nested GROUP BY SQLLogicTest was not necessary. The existing nested_float_edge_cases_match_rows_fallback unit test directly exercises GroupValuesColumn::intern with nested +0.0 and -0.0 values and verifies that its grouping matches the established GroupValuesRows fallback. Since that test predates this change, it provides regression coverage for the path being modified here. An end-to-end SQLLogicTest could still be useful additional coverage, but I don't think it is required for this PR.

Thanks for addressing the feedback!

@kosiew

kosiew commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

🚀
@advitrocks9
Thank you for your contribution.

@kosiew
kosiew added this pull request to the merge queue Sep 22, 2026
Merged via the queue into apache:main with commit d42cd85 Sep 22, 2026
42 of 43 checks passed
Omega359 pushed a commit to Omega359/arrow-datafusion that referenced this pull request Oct 11, 2026
## Which issue does this PR close?

No issue filed. Part of #15914, which tracks the Spark function library.

## Rationale for this change

`equal_null` is missing, and the `misc` module it belongs in had no
functions in it at all. In
Spark it is an alias: `EqualNull(l, r)` is rewritten to `EqualNullSafe`,
the `<=>` operator, so
it is null-safe equality. Both NULL is true, and the result is never
NULL:

```sql
SELECT equal_null(NULL, NULL);              -- true
SELECT equal_null(NULL::int, 1::int);       -- false
```

DataFusion already has that operator as `IS NOT DISTINCT FROM`, so this
mirrors Spark's own
structure instead of writing a second comparison kernel.

## What changes are included in this PR?

`misc/equal_null.rs` and its registration. `simplify()` rewrites to the
operator, which is
Spark's `replacement` field, and `invoke_with_args` calls `apply_cmp`
with the same operator so
the function also works with the logical optimizer disabled, as the
crate README requires for
Comet.

Two of the stub's commented-out queries were malformed, the porting
script wrote one cast per
distinct `typeof()` key, so a call with two identical literals lost an
argument. The same
artifact affects 10 more pairs in 6 other spark files, left alone here.

Two divergences from Spark are left alone because they belong to the
operator, not to this
function. Spark treats `-NaN` and `NaN` as equal and DataFusion does
not, which is true of
`IS NOT DISTINCT FROM` generally. Spark also rejects maps at analysis
since `MapType` is not
orderable, while `comparison_coercion` here accepts them.

`return_field_from_args` is overridden so the field is not nullable.
`normalize_float_zero` now
recurses into nested children. `compare_op_for_nested` and
`GroupValuesColumn` both normalize
through it, so the scalar kernels and the two group-by paths agree on
nested float keys.

## Are these changes tested?

Yes, `spark/misc/equal_null.slt` goes from a skipped stub to 27
assertions covering the truth
table, float ordering, columns, arrays, structs, decimals and the arity
errors.

Reverting `simplify()` to plain `Eq` fails 9 of them, so the file is not
passing on constant
folding. Putting `invoke_with_args` back to a stub fails the two queries
that run with the
optimizer off, and nothing else.

## Are there any user-facing changes?

Yes. `equal_null` is new in `datafusion-spark`, and `-0.0` now matches
`+0.0` inside nested
values, in comparisons and in `GROUP BY`, matching the scalar kernels.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate spark Changes to datafusion-spark crate sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants