Skip to content

feat: warn on NULL equality predicates#22948

Open
ametel01 wants to merge 3 commits into
apache:mainfrom
ametel01:issue-14434-null-diagnostic-warning
Open

feat: warn on NULL equality predicates#22948
ametel01 wants to merge 3 commits into
apache:mainfrom
ametel01:issue-14434-null-diagnostic-warning

Conversation

@ametel01

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

SQL comparisons such as expr = NULL and expr <> NULL evaluate to NULL under SQL three-valued logic, not to true or false. When those comparisons appear in predicate contexts such as WHERE, JOIN ON, or HAVING, they are almost always a user mistake where IS NULL or IS NOT NULL was intended.

This PR emits non-fatal diagnostic warnings for those cases so callers that surface Diagnostic information can help users find and fix the query without changing planning success or query semantics.

What changes are included in this PR?

  • Adds warning collection to SqlToRel, with SqlToRel::take_warnings() to drain non-fatal planning diagnostics after planning.
  • Adds predicate-scoped detection for = NULL and <> NULL comparisons.
  • Wires detection into WHERE, JOIN ON, and HAVING planning paths.
  • Recursively checks predicate expression structure such as nested AND / OR binary predicates and CASE WHEN conditions, without warning for projection-only expressions like SELECT col = NULL.
  • Creates Diagnostic::new_warning entries with a primary span on the comparison expression and help pointing at the NULL literal when spans are available.

Are these changes tested?

Yes. Added regression coverage in datafusion/sql/tests/cases/diagnostic.rs for:

  • WHERE col = NULL
  • WHERE NULL = col
  • WHERE col <> NULL
  • JOIN ... ON col = NULL
  • HAVING ... = NULL
  • nested CASE WHEN col = NULL inside a predicate
  • no warning for IS NULL
  • no warning for projection-only SELECT col = NULL
  • multiple warnings in one predicate

Validation run locally:

cargo fmt --all
cargo test -p datafusion-sql --test sql_integration diagnostic
cargo test -p datafusion-sql
cargo clippy -p datafusion-sql --all-targets --all-features -- -D warnings
cargo clippy --all-targets --all-features -- -D warnings
git diff main..HEAD --check

Are there any user-facing changes?

Yes, but non-breaking. SQL planning can now collect warning diagnostics for likely mistaken NULL equality predicates. Consumers can retrieve them with SqlToRel::take_warnings() and decide how to present them. Planning still succeeds and query semantics are unchanged.

@github-actions github-actions Bot added the sql SQL Planner label Jun 15, 2026
@ametel01 ametel01 marked this pull request as ready for review June 15, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Emit warning with attached Diagnostic when doing = NULL

1 participant