This is an automated email from the ASF dual-hosted git repository.
HyukjinKwon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new a6611583e82b [SPARK-60576][SQL] Parenthesize NOT operands in connector
comparisons
a6611583e82b is described below
commit a6611583e82b2bae51ea2cda98d24c215ed93da7
Author: Linhong Liu <[email protected]>
AuthorDate: Wed Oct 28 10:25:55 2026 -0700
[SPARK-60576][SQL] Parenthesize NOT operands in connector comparisons
### What changes were proposed in this pull request?
Parenthesize NOT operands in inputToSQL, with a comment explaining why the
existing child-count check misses this unary operator.
### Why are the changes needed?
NOT has lower SQL precedence than comparisons. Ungrouped operands also
change the IS NULL checks synthesized for null-safe equality. For example,
`(NOT a) <=> b` with `a = true` and `b = false` should return true, but the
ungrouped generated expression returns false.
### Does this PR introduce _any_ user-facing change?
Yes, a bug fix. Connector comparison SQL retains the intended grouping of
NOT, including the expansion of null-safe equality.
### How was this patch tested?
Added SQL grouping assertions and 189 evaluations spanning all seven
comparison operators, NOT on either or both sides, and every TRUE/FALSE/NULL
input combination. The semantic regression fails on unchanged Apache code: (NOT
a) <=> b with a=true and b=false returns false instead of true. On JDK 17, all
22 V2PredicateSuite tests and three JDBC filter/grouping tests passed. Catalyst
Java Checkstyle, SQL test Scalastyle and git diff --check passed.
```bash
build/sbt 'sql/testOnly
org.apache.spark.sql.execution.datasources.v2.V2PredicateSuite'
build/sbt 'sql/testOnly org.apache.spark.sql.jdbc.JDBCSuite -- -z "compile
filters" -z "IS [NOT] NULL"'
```
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-6)
Closes #59816 from linhongliu-db/fix-not-operand-precedence.
Authored-by: Linhong Liu <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
---
.../sql/connector/util/V2ExpressionSQLBuilder.java | 5 ++-
.../datasources/v2/V2PredicateSuite.scala | 46 ++++++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)
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 9d80bf730e48..17d54364af0b 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
@@ -288,7 +288,10 @@ public class V2ExpressionSQLBuilder {
}
protected String inputToSQL(Expression input) {
- if (input.children().length > 1) {
+ // NOT is unary, so the arity check alone misses it. Its lower precedence
requires
+ // parentheses in comparisons and the IS NULL checks synthesized for
null-safe equality.
+ if (input.children().length > 1 ||
+ (input instanceof GeneralScalarExpression e &&
e.name().equals("NOT"))) {
return "(" + build(input) + ")";
} else {
return build(input);
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2PredicateSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2PredicateSuite.scala
index 96d9f998b64c..7396687d9020 100644
---
a/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2PredicateSuite.scala
+++
b/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2PredicateSuite.scala
@@ -105,6 +105,52 @@ class V2PredicateSuite extends SparkFunSuite {
assert(PredicateUtils.toV1(predicate1).get.toV2 == predicate1)
}
+ for ((left, right, leftSql, rightSql) <- Seq(
+ (new Not(new Predicate("BOOLEAN_EXPRESSION",
Array[Expression](ref("a")))),
+ ref("b"), "(NOT (a))", "b"),
+ (ref("a"), new Not(new Predicate("BOOLEAN_EXPRESSION",
Array[Expression](ref("b")))),
+ "a", "(NOT (b))"),
+ (new Not(new Predicate("BOOLEAN_EXPRESSION",
Array[Expression](ref("a")))),
+ new Not(new Predicate("BOOLEAN_EXPRESSION",
Array[Expression](ref("b")))),
+ "(NOT (a))", "(NOT (b))"))) {
+ test(s"comparisons preserve NOT operand precedence: $leftSql, $rightSql") {
+ val comparison = new Predicate("<=>", Array[Expression](left, right))
+ assert(comparison.describe() ==
+ s"(($leftSql IS NOT NULL AND $rightSql IS NOT NULL AND $leftSql =
$rightSql) " +
+ s"OR ($leftSql IS NULL AND $rightSql IS NULL))")
+ assert(new Predicate("=", Array[Expression](left, right)).describe() ==
+ s"$leftSql = $rightSql")
+ }
+ }
+
+ test("comparisons with NOT preserve three-valued logic") {
+ def evaluate(sql: String, a: java.lang.Boolean, b: java.lang.Boolean): Any
= {
+ CatalystSqlParser.parseExpression(sql).transform {
+ case attr: UnresolvedAttribute =>
+ CatalystLiteral.create(if (attr.name == "a") a else b, BooleanType)
+ }.eval()
+ }
+
+ val values = Seq[java.lang.Boolean](true, false, null)
+ val notA = new Not(new Predicate("BOOLEAN_EXPRESSION",
Array[Expression](ref("a"))))
+ val notB = new Not(new Predicate("BOOLEAN_EXPRESSION",
Array[Expression](ref("b"))))
+ for {
+ (left, right, leftSql, rightSql) <- Seq(
+ (notA, ref("b"), "(NOT a)", "b"),
+ (ref("a"), notB, "a", "(NOT b)"),
+ (notA, notB, "(NOT a)", "(NOT b)"))
+ operator <- Seq("<=>", "=", "<>", "<", "<=", ">", ">=")
+ a <- values
+ b <- values
+ } {
+ val predicate = new Predicate(operator, Array[Expression](left, right))
+ val expected = s"$leftSql $operator $rightSql"
+ withClue(s"${predicate.describe()}, a=$a, b=$b: ") {
+ assert(evaluate(predicate.describe(), a, b) == evaluate(expected, a,
b))
+ }
+ }
+ }
+
test("EqualNullSafe") {
val predicate1 = new Predicate("<=>", Array[Expression](ref("a"),
LiteralValue(1, IntegerType)))
val predicate2 = new Predicate("<=>", Array[Expression](ref("a"),
LiteralValue(1, IntegerType)))
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
