This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 4a330ce9254a [SPARK-53454][SQL][FOLLOWUP] Parenthesize
AlwaysTrue/AlwaysFalse SQL so it is valid as a nested operand
4a330ce9254a is described below
commit 4a330ce9254a8b0f5cd42109e3757fcef01a2a04
Author: Wenchen Fan <[email protected]>
AuthorDate: Tue Jun 2 19:46:18 2026 +0800
[SPARK-53454][SQL][FOLLOWUP] Parenthesize AlwaysTrue/AlwaysFalse SQL so it
is valid as a nested operand
### What changes were proposed in this pull request?
Followup to https://github.com/apache/spark/pull/56085.
That PR made `JDBCSQLBuilder.build()` emit `"1 = 1"` / `"1 = 0"` for
`AlwaysTrue` / `AlwaysFalse` predicates. This change parenthesizes the output
-- `"(1 = 1)"` / `"(1 = 0)"` -- so it remains valid SQL when the predicate is
nested as an operand of a larger expression.
### Why are the changes needed?
`AlwaysTrue` / `AlwaysFalse` can appear not only as a standalone `WHERE`
predicate but also nested as an operand of a larger expression (e.g. when an
equality against a boolean column is expanded to a null-aware form). With the
bare `1 = 1`, the generated SQL inlines into invalid syntax such as `"a" = 1 =
1` or `1 = 1 IS NOT NULL`, which databases reject (e.g. PostgreSQL: `ERROR:
syntax error at or near "="`). Parenthesizing produces `"a" = (1 = 1)` / `(1 =
1) IS NOT NULL`, which is v [...]
### Does this PR introduce _any_ user-facing change?
Yes. JDBC pushed-filter SQL for `AlwaysTrue` / `AlwaysFalse` now uses `(1 =
1)` / `(1 = 0)` instead of bare `1 = 1` / `1 = 0`. This is semantically
identical and fixes queries that previously generated invalid SQL when these
predicates were nested.
### How was this patch tested?
Extended the existing unit test in `JDBCSuite` to also cover the
nested-operand case (`"a" = (1 = 1)`). Verified the test fails without the fix
(`"1 = 1"` not equal to `"(1 = 1)"`) and passes with it.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
Closes #56263 from cloud-fan/SPARK-53454-followup.
Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 62cbb3ac4e47a2392cbb7b42cac1045be1d50f9b)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../scala/org/apache/spark/sql/jdbc/JdbcDialects.scala | 7 +++++--
.../scala/org/apache/spark/sql/jdbc/JDBCSuite.scala | 17 ++++++++++++++---
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
index 3b5182721c29..a34d23512e99 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
@@ -402,9 +402,12 @@ abstract class JdbcDialect extends Serializable with
Logging {
private[jdbc] class JDBCSQLBuilder extends V2ExpressionSQLBuilder {
// SPARK-53454: Produce portable SQL for AlwaysTrue/AlwaysFalse predicates.
// Some databases (Oracle, DB2) do not support bare TRUE/FALSE in WHERE
clauses.
+ // The result is parenthesized so it stays valid when nested as an operand
of a
+ // larger expression (e.g. "a" = (1 = 1) or (1 = 1) IS NOT NULL), not just
as a
+ // standalone WHERE predicate.
override def build(expr: Expression): String = expr match {
- case _: AlwaysTrue => "1 = 1"
- case _: AlwaysFalse => "1 = 0"
+ case _: AlwaysTrue => "(1 = 1)"
+ case _: AlwaysFalse => "(1 = 0)"
case _ => super.build(expr)
}
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 e3263c3cf357..4a7771305994 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
@@ -35,7 +35,8 @@ import org.apache.spark.sql.catalyst.{analysis,
TableIdentifier}
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
import org.apache.spark.sql.catalyst.plans.logical.ShowCreateTable
import org.apache.spark.sql.catalyst.util.{CaseInsensitiveMap,
CharVarcharUtils, DateTimeTestUtils}
-import org.apache.spark.sql.connector.expressions.filter.{AlwaysFalse,
AlwaysTrue}
+import org.apache.spark.sql.connector.expressions.{Expression => V2Expression,
FieldReference}
+import org.apache.spark.sql.connector.expressions.filter.{AlwaysFalse,
AlwaysTrue, Predicate}
import org.apache.spark.sql.execution.{DataSourceScanExec, ExtendedMode,
ProjectExec}
import org.apache.spark.sql.execution.command.{ExplainCommand,
ShowCreateTableCommand}
import org.apache.spark.sql.execution.datasources.{LogicalRelation,
LogicalRelationWithTable}
@@ -895,8 +896,18 @@ class JDBCSuite extends SharedSparkSession {
test("SPARK-53454: AlwaysTrue/AlwaysFalse compile to portable SQL in
JDBCSQLBuilder") {
val dialect = JdbcDialects.get("jdbc:")
- assert(dialect.compileExpression(new AlwaysTrue).get === "1 = 1")
- assert(dialect.compileExpression(new AlwaysFalse).get === "1 = 0")
+ assert(dialect.compileExpression(new AlwaysTrue).get === "(1 = 1)")
+ assert(dialect.compileExpression(new AlwaysFalse).get === "(1 = 0)")
+
+ // The result must stay valid when AlwaysTrue/AlwaysFalse is nested as an
operand
+ // of a larger expression, not just as a standalone WHERE predicate.
Without the
+ // surrounding parentheses the bare `1 = 1` would inline into invalid SQL
such as
+ // `a = 1 = 1`.
+ val ref = FieldReference("a")
+ val eqTrue = new Predicate("=", Array[V2Expression](ref, new AlwaysTrue))
+ val eqFalse = new Predicate("=", Array[V2Expression](ref, new AlwaysFalse))
+ assert(dialect.compileExpression(eqTrue).get === "\"a\" = (1 = 1)")
+ assert(dialect.compileExpression(eqFalse).get === "\"a\" = (1 = 0)")
}
test("Dialect unregister") {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
