ametel01 opened a new pull request, #22948: URL: https://github.com/apache/datafusion/pull/22948
## Which issue does this PR close? - Closes #14434. ## 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: ```shell 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. -- 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]
