This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 2d6c53b2cdc3 [SPARK-57988][SQL] Parenthesize IN and other
non-comparison predicate operands of IS [NOT] NULL in V2ExpressionSQLBuilder
2d6c53b2cdc3 is described below
commit 2d6c53b2cdc3060618ff306275658df3d4676fdc
Author: Joel Robin P <[email protected]>
AuthorDate: Thu Jul 9 22:07:36 2026 +0800
[SPARK-57988][SQL] Parenthesize IN and other non-comparison predicate
operands of IS [NOT] NULL in V2ExpressionSQLBuilder
### 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 change [...]
### 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.
Closes #57080 from joelrobin18/SPARK-57988-isnull-predicate-operand-parens.
Authored-by: Joel Robin P <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 4b3765b794c341cad5f5074cd4e33a9600b4c45e)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala | 24 +++++++++
.../sql/connector/util/V2ExpressionSQLBuilder.java | 23 ++++++--
.../apache/spark/sql/jdbc/MsSqlServerDialect.scala | 14 +++--
.../org/apache/spark/sql/jdbc/JDBCSuite.scala | 63 ++++++++++++++++++++++
.../org/apache/spark/sql/jdbc/JDBCV2Suite.scala | 15 ++++++
5 files changed, 127 insertions(+), 12 deletions(-)
diff --git
a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
index 8e04660686ed..6f2a9e97ff00 100644
---
a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
+++
b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
@@ -737,6 +737,30 @@ private[v2] trait V2JDBCTest
}
}
+ test("SPARK-57988: IS [NOT] NULL over an IN operand is pushed down") {
+ val tbl = s"$catalogName.in_is_null"
+ withTable(tbl) {
+ // b is a string label so the result type is the same across dialects
(an INT column
+ // comes back as BigDecimal on some engines such as Oracle).
+ sql(s"CREATE TABLE $tbl (a INT, b VARCHAR(8))")
+ Seq[(Option[Int], String)]((Some(10000), "x"), (Some(20000), "y"),
(None, "z"))
+ .toDF("a", "b").write.insertInto(tbl)
+
+ // An IN operand must be parenthesized as `(a IN (...)) IS [NOT] NULL`;
the unwrapped
+ // rendering `a IN (...) IS NOT NULL` is a syntax error on engines such
as PostgreSQL.
+ // The NULL row exercises the IS NULL branch: (NULL IN (...)) is NULL,
so it matches
+ // IS NULL and not IS NOT NULL. Dialects without a boolean type (e.g.
SQL Server) cannot
+ // push this, but Spark evaluates it and returns the same rows.
+ val df1 = sql(s"SELECT b FROM $tbl WHERE (a IN (10000, 20000)) IS NOT
NULL")
+ checkFilterPushed(df1, supportsIsNullOverPredicate)
+ assert(df1.collect().map(_.getString(0)).sorted === Array("x", "y"))
+
+ val df2 = sql(s"SELECT b FROM $tbl WHERE (a IN (10000, 20000)) IS NULL")
+ checkFilterPushed(df2, supportsIsNullOverPredicate)
+ assert(df2.collect().map(_.getString(0)) === Array("z"))
+ }
+ }
+
test("SPARK-57040: TABLESAMPLE with replacement is not pushed down") {
withTable(s"$catalogName.new_table") {
sql(s"CREATE TABLE $catalogName.new_table (col1 INT, col2 INT)")
diff --git
a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
index eaba2aa79df5..3ceb131d2cec 100644
---
a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
+++
b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
@@ -211,7 +211,7 @@ public class V2ExpressionSQLBuilder {
return joinListToString(list, ", ", v + " IN (", ")");
}
- // The binary comparison operators, kept in one place so build,
visitIsNullOperand and
+ // The binary comparison operators, kept in one place so build,
isNullOperandNeedsParens and
// dialect overrides stay in sync.
protected boolean isBinaryComparisonOperator(String name) {
return switch (name) {
@@ -220,10 +220,25 @@ public class V2ExpressionSQLBuilder {
};
}
- // Parenthesize a binary comparison operand so `col = 'x' IS NULL` renders as
- // `(col = 'x') IS NULL`. Dialects such as Snowflake bind IS NULL tighter
than =.
+ // Operators whose rendered SQL is NOT a self-delimiting primary and
therefore must be
+ // parenthesized before a trailing IS [NOT] NULL: binary comparisons (per
SPARK-57243) plus IN,
+ // the boolean connectives, LIKE-family, and arithmetic. Function calls,
CASE and CAST already
+ // render self-delimited (`f(...)`, `CASE ... END`, `CAST(...)`), so they
are intentionally left
+ // unwrapped to avoid needlessly changing the generated SQL for cases that
were already valid.
+ protected boolean isNullOperandNeedsParens(String name) {
+ return isBinaryComparisonOperator(name) || switch (name) {
+ case "IN", "NOT", "AND", "OR", "STARTS_WITH", "ENDS_WITH", "CONTAINS",
+ "+", "-", "*", "/", "%", "&", "|", "^", "~" -> true;
+ default -> false;
+ };
+ }
+
+ // Parenthesize an operand of IS [NOT] NULL whose rendered SQL is not
self-delimiting, so
+ // `col = 'x' IS NULL` renders as `(col = 'x') IS NULL` and `a IN (1, 2) IS
NULL` renders as
+ // `(a IN (1, 2)) IS NULL`. This keeps the output valid and preserves the
intended precedence
+ // across dialects, some of which (e.g. Snowflake) bind IS NULL tighter than
comparison operators.
protected String visitIsNullOperand(Expression operand) {
- if (operand instanceof GeneralScalarExpression e &&
isBinaryComparisonOperator(e.name())) {
+ if (operand instanceof GeneralScalarExpression e &&
isNullOperandNeedsParens(e.name())) {
return "(" + build(operand) + ")";
}
return build(operand);
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
index 5cfec0cdcbd4..9c30b73bb042 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
@@ -100,19 +100,17 @@ private case class MsSqlServerDialect() extends
JdbcDialect with NoLegacyJDBCErr
case _ => super.visitSQLFunction(funcName, inputs)
}
- private def isBinaryComparison(e: Expression): Boolean = e match {
- case p: Predicate => isBinaryComparisonOperator(p.name())
- case _ => false
- }
-
- // MsSqlServer has no boolean type: a comparison expression cannot appear
as a value.
- // IS NULL / IS NOT NULL over a comparison is rejected so Spark evaluates
it locally.
+ // MsSqlServer has no boolean type: a predicate (comparison, IN,
AND/OR/NOT, LIKE-family, ...)
+ // cannot appear as a value. IS NULL / IS NOT NULL over any predicate
operand is rejected so
+ // Spark evaluates it locally. Rewriting the operand to a bit value
(predicateToIntSQL) is not
+ // an option because IIF(p, 1, 0) is never NULL, which would change the
result. Non-predicate
+ // operands such as arithmetic are still pushed down.
// Binary comparisons themselves are pushed down via inputToSQLNoBool,
which rewrites any
// boolean sub-expressions into bit-typed equivalents.
override def build(expr: Expression): String = {
expr match {
case e: Predicate => e.name() match {
- case "IS_NULL" | "IS_NOT_NULL" if
isBinaryComparison(e.children().head) =>
+ case "IS_NULL" | "IS_NOT_NULL" if
e.children().head.isInstanceOf[Predicate] =>
visitUnexpectedExpr(expr)
case "=" | "<>" | "<=>" | "<" | "<=" | ">" | ">=" =>
val Array(l, r) = e.children().map(inputToSQLNoBool)
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
index 74f803a5a173..ebf1ce36dd56 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
@@ -1074,6 +1074,69 @@ class JDBCSuite extends SharedSparkSession {
assert(msSqlServer.compileExpression(bareIsNull).get === "\"a\" IS NULL")
}
+ test("SPARK-57988: IS [NOT] NULL parenthesizes IN and other non-comparison
predicate operands") {
+ val dialect = JdbcDialects.get("jdbc:")
+ val h2 = JdbcDialects.get("jdbc:h2:mem:testdb0")
+ val msSqlServer = JdbcDialects.get("jdbc:sqlserver://127.0.0.1/db")
+ val a = FieldReference("a")
+ val b = FieldReference("b")
+ val one = LiteralValue(1, IntegerType)
+ val two = LiteralValue(2, IntegerType)
+
+ // An IN operand must be parenthesized: `"a" IN (1, 2) IS NULL` is invalid
SQL (PostgreSQL,
+ // for example, rejects it), and BooleanSimplification produces
IsNotNull(In(...)) from
+ // `x IN (...) OR x NOT IN (...)`. MsSqlServer has no boolean type, so IS
[NOT] NULL over any
+ // predicate operand is not pushed down there at all.
+ val in = new Predicate("IN", Array[V2Expression](a, one, two))
+ for ((isNullOp, keyword) <- Seq("IS_NULL" -> "IS NULL", "IS_NOT_NULL" ->
"IS NOT NULL")) {
+ val expr = new Predicate(isNullOp, Array[V2Expression](in))
+ assert(dialect.compileExpression(expr).get === s"""("a" IN (1, 2))
$keyword""")
+ assert(msSqlServer.compileExpression(expr).isEmpty)
+ }
+
+ // The boolean connectives are parenthesized as well.
+ val eqA = new Predicate("=", Array[V2Expression](a, one))
+ val eqB = new Predicate("=", Array[V2Expression](b, two))
+ val and = new Predicate("AND", Array[V2Expression](eqA, eqB))
+ val or = new Predicate("OR", Array[V2Expression](eqA, eqB))
+ val not = new Predicate("NOT", Array[V2Expression](eqA))
+ assert(dialect.compileExpression(new Predicate("IS_NULL",
Array[V2Expression](and))).get ===
+ """(("a" = 1) AND ("b" = 2)) IS NULL""")
+ assert(dialect.compileExpression(new Predicate("IS_NOT_NULL",
Array[V2Expression](or))).get ===
+ """(("a" = 1) OR ("b" = 2)) IS NOT NULL""")
+ assert(dialect.compileExpression(new Predicate("IS_NULL",
Array[V2Expression](not))).get ===
+ """(NOT ("a" = 1)) IS NULL""")
+ Seq(and, or, not).foreach { p =>
+ val isNull = new Predicate("IS_NULL", Array[V2Expression](p))
+ assert(msSqlServer.compileExpression(isNull).isEmpty)
+ }
+
+ // LIKE-family operators render with a trailing ESCAPE clause and must be
delimited too.
+ // LiteralValue for StringType must use UTF8String (Spark's internal
string type).
+ import org.apache.spark.unsafe.types.UTF8String
+ for ((op, pattern) <- Seq(
+ "STARTS_WITH" -> "abc%", "ENDS_WITH" -> "%abc", "CONTAINS" ->
"%abc%")) {
+ val like = new Predicate(op,
+ Array[V2Expression](a, LiteralValue(UTF8String.fromString("abc"),
StringType)))
+ val isNullLike = new Predicate("IS_NULL", Array[V2Expression](like))
+ assert(dialect.compileExpression(isNullLike).get ===
+ raw"""("a" LIKE '$pattern' ESCAPE '\') IS NULL""")
+ assert(msSqlServer.compileExpression(isNullLike).isEmpty)
+ }
+
+ // Arithmetic operands are parenthesized; they are value expressions, not
predicates, so
+ // MsSqlServer still pushes them down.
+ val plus = new GeneralScalarExpression("+", Array[V2Expression](a, b))
+ val isNullPlus = new Predicate("IS_NULL", Array[V2Expression](plus))
+ assert(dialect.compileExpression(isNullPlus).get === """("a" + "b") IS
NULL""")
+ assert(msSqlServer.compileExpression(isNullPlus).get === """("a" + "b") IS
NULL""")
+
+ // Self-delimiting operands are left unwrapped: function calls render as
`f(...)` already.
+ val abs = new GeneralScalarExpression("ABS", Array[V2Expression](a))
+ assert(h2.compileExpression(new Predicate("IS_NULL",
Array[V2Expression](abs))).get ===
+ """ABS("a") IS NULL""")
+ }
+
test("SPARK-57332: escape backslash in LIKE pattern for
STARTS_WITH/ENDS_WITH/CONTAINS") {
// Default dialect: standard SQL string literals take backslash verbatim,
so the LIKE escape
// character `\` appears once in the ESCAPE clause and a literal backslash
in the value is
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
index 742a895a2839..dd3d90d2df05 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala
@@ -286,6 +286,21 @@ class JDBCV2Suite extends SharedSparkSession with
ExplainSuiteHelper {
checkAnswer(df2, Seq.empty)
}
+ test("SPARK-57988: IS [NOT] NULL over an IN operand is pushed down and runs
on H2") {
+ // Without the parentheses around the IN operand the pushed SQL would be
+ // `SALARY IN (...) IS NOT NULL`, which databases reject as a syntax error.
+ val df1 =
+ sql("SELECT name FROM h2.test.employee WHERE (salary IN (10000, 12000))
IS NOT NULL")
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [(SALARY IN (10000.00, 12000.00)) IS
NOT NULL]")
+ checkAnswer(df1, Seq(Row("amy"), Row("alex"), Row("cathy"), Row("david"),
Row("jen")))
+
+ val df2 = sql("SELECT name FROM h2.test.employee WHERE (salary IN (10000,
12000)) IS NULL")
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [(SALARY IN (10000.00, 12000.00)) IS
NULL]")
+ checkAnswer(df2, Seq.empty)
+ }
+
// TABLESAMPLE ({integer_expression | decimal_expression} PERCENT) and
// TABLESAMPLE (BUCKET integer_expression OUT OF integer_expression)
// are tested in JDBC dialect tests because TABLESAMPLE is not supported by
all the DBMS
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
