wilbyang opened a new pull request, #20739:
URL: https://github.com/apache/datafusion/pull/20739
## Which issue does this PR close?
Closes #14434
## Rationale for this change
`expr = NULL` and `expr <> NULL` always evaluate to `NULL` (never `TRUE` or
`FALSE`) due to SQL's three-valued logic. This is almost always a bug — the
user almost certainly meant `IS NULL` or `IS NOT NULL`. DataFusion currently
executes these queries silently with no feedback.
## What changes are included in this PR?
### Warning infrastructure (`datafusion/sql/src/planner.rs`)
Added `warnings: Arc<Mutex<Vec<Diagnostic>>>` to `PlannerContext` with two
new public methods:
- `add_warning(diag: Diagnostic)` — push a non-fatal warning during planning
- `take_warnings() -> Vec<Diagnostic>` — drain all collected warnings after
planning
Using `Arc` ensures all clones of `PlannerContext` (e.g. for subqueries)
share the same backing store, so warnings from nested queries surface to the
top-level caller. This is the **first use** of `Diagnostic::new_warning` in the
codebase and lays the foundation for future non-fatal diagnostics.
### Detection (`datafusion/sql/src/expr/mod.rs`)
In the binary expression stack machine, detects `= NULL` / `<> NULL` on
either operand and emits a `Diagnostic::new_warning`:
```
'= NULL' will always be NULL (null comparisons never return true or false)
help: use 'IS NULL' instead
```
The span points at the non-null operand (the column/expression), since NULL
literals carry no span.
### Tests (`datafusion/sql/tests/cases/diagnostic.rs`)
Added a `do_query_warnings` helper and 5 new tests:
| Test | Expectation |
|---|---|
| `WHERE col = NULL` | 1 warning, span on `col`, suggests `IS NULL` |
| `WHERE NULL = col` | 1 warning, span on `col`, suggests `IS NULL` |
| `WHERE col <> NULL` | 1 warning, span on `col`, suggests `IS NOT NULL` |
| `WHERE col IS NULL` | 0 warnings (correct form) |
| `WHERE a = NULL AND b = NULL` | 2 warnings |
## How callers consume warnings
```rust
let mut planner_context = PlannerContext::new();
let plan = sql_to_rel
.sql_statement_to_plan_with_context(stmt, &mut planner_context)?;
let warnings = planner_context.take_warnings(); // Vec<Diagnostic>
```
## Are these changes tested?
Yes — 5 new tests, all passing. Full `datafusion-sql` test suite passes (459
+ 69 + 12 tests).
--
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]
