joelrobin18 opened a new pull request, #57080:
URL: https://github.com/apache/spark/pull/57080
### What changes were proposed in this pull request?
`V2ExpressionSQLBuilder` currently parenthesizes the operand of `IS [NOT]
NULL` only when it is a binary comparison (added in SPARK-57243 via
`visitIsNullOperand`). Any other non-self-delimiting operand is rendered
unwrapped, producing invalid or misparsed SQL, e.g. for an `IN` predicate:
```sql
-- before (invalid: PostgreSQL reports "syntax error at or near NOT")
"a" IN (1, 2) IS NOT NULL
-- after
("a" IN (1, 2)) IS NOT NULL
```
This PR introduces an `isNullOperandNeedsParens()` hook that extends the
parenthesization to every operator whose rendered SQL is not a self-delimiting
primary:
- binary comparisons (`=`, `<>`, `<=>`, `<`, `<=`, `>`, `>=`) -- as before
(SPARK-57243)
- `IN`
- boolean connectives (`AND`, `OR`, `NOT`) -- without parens, `IS NULL`
binds to only part of the operand (e.g. `NOT ("a" = 1) IS NULL` parses as `NOT
(("a" = 1) IS NULL)`), silently changing results
- LIKE-family (`STARTS_WITH`, `ENDS_WITH`, `CONTAINS`) -- the rendering ends
with `ESCAPE '\'`, which collides with a trailing `IS NULL`
- arithmetic (`+`, `-`, `*`, `/`, `%`, `&`, `|`, `^`, `~`) -- defensive, for
dialects with nonstandard `IS NULL` precedence
Function calls, `CASE ... END` and `CAST(...)` already render self-delimited
and are intentionally left unwrapped, preserving SPARK-57243's rationale of not
changing SQL that was already valid.
`MsSqlServerDialect` is also updated: its guard that rejects `IS [NOT] NULL`
over a binary comparison (T-SQL has no boolean value type, so a predicate
cannot appear as an `IS NULL` operand at all -- parentheses cannot fix this) is
widened to reject any predicate operand. `compileExpression` then returns
`None` and Spark evaluates the filter locally instead of pushing SQL that is
guaranteed to fail at runtime. Non-predicate operands such as arithmetic are
still pushed down.
### Why are the changes needed?
Catalyst's `BooleanSimplification` rewrites `col IN (...) OR col NOT IN
(...)` into `IsNotNull(In(col, ...))`, so `IS [NOT] NULL` over an `IN`
predicate reaches V2 pushdown even though the user never wrote it. The
generated SQL is rejected by the external database (e.g. PostgreSQL: `syntax
error at or near "NOT"`), failing the whole Spark query.
### Does this PR introduce _any_ user-facing change?
Yes, but only as a bug fix. Previously, queries whose pushed filters
contained `IS [NOT] NULL` over an `IN` (or other non-comparison) predicate
failed with a syntax error from the external database (e.g. PostgreSQL: `syntax
error at or near "NOT"`). With this fix the generated SQL is valid and such
queries succeed. On SQL Server, where this construct cannot be expressed at
all, the filter is no longer pushed down and Spark evaluates it locally, so
those queries also succeed. No changes for queries that were already working.
### How was this patch tested?
- New test in `JDBCSuite` covering the rendered SQL for `IN`,
`AND`/`OR`/`NOT`, LIKE-family, arithmetic and function-call operands of `IS
[NOT] NULL`, including the MsSqlServer no-pushdown and still-pushed-down cases.
Verified it fails on master without the fix (`"a" IN (1, 2) IS NULL` vs `("a"
IN (1, 2)) IS NULL`).
- New end-to-end test in `JDBCV2Suite` verifying `(salary IN (10000, 12000))
IS [NOT] NULL` is pushed down and executes correctly on H2.
- New test in `V2JDBCTest` (docker integration suites) verifying pushdown
and results on real databases, reusing the `supportsIsNullOverPredicate` hook
from SPARK-57243.
- Ran `JDBCSuite`, `JDBCV2Suite`, `V2PredicateSuite` and
`DataSourceV2StrategySuite` (252 tests, all pass).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
This pull request and its description were written by Isaac.
--
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]
