Skip to content

Commit 62cbb3a

Browse files
committed
[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 #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 valid both standalone and nested. ### 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 <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
1 parent 1b60970 commit 62cbb3a

2 files changed

Lines changed: 19 additions & 5 deletions

File tree

sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,9 +402,12 @@ abstract class JdbcDialect extends Serializable with Logging {
402402
private[jdbc] class JDBCSQLBuilder extends V2ExpressionSQLBuilder {
403403
// SPARK-53454: Produce portable SQL for AlwaysTrue/AlwaysFalse predicates.
404404
// Some databases (Oracle, DB2) do not support bare TRUE/FALSE in WHERE clauses.
405+
// The result is parenthesized so it stays valid when nested as an operand of a
406+
// larger expression (e.g. "a" = (1 = 1) or (1 = 1) IS NOT NULL), not just as a
407+
// standalone WHERE predicate.
405408
override def build(expr: Expression): String = expr match {
406-
case _: AlwaysTrue => "1 = 1"
407-
case _: AlwaysFalse => "1 = 0"
409+
case _: AlwaysTrue => "(1 = 1)"
410+
case _: AlwaysFalse => "(1 = 0)"
408411
case _ => super.build(expr)
409412
}
410413

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ import org.apache.spark.sql.catalyst.{analysis, TableIdentifier}
3535
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
3636
import org.apache.spark.sql.catalyst.plans.logical.ShowCreateTable
3737
import org.apache.spark.sql.catalyst.util.{CaseInsensitiveMap, CharVarcharUtils, DateTimeTestUtils}
38-
import org.apache.spark.sql.connector.expressions.filter.{AlwaysFalse, AlwaysTrue}
38+
import org.apache.spark.sql.connector.expressions.{Expression => V2Expression, FieldReference}
39+
import org.apache.spark.sql.connector.expressions.filter.{AlwaysFalse, AlwaysTrue, Predicate}
3940
import org.apache.spark.sql.execution.{DataSourceScanExec, ExtendedMode, ProjectExec}
4041
import org.apache.spark.sql.execution.command.{ExplainCommand, ShowCreateTableCommand}
4142
import org.apache.spark.sql.execution.datasources.{LogicalRelation, LogicalRelationWithTable}
@@ -895,8 +896,18 @@ class JDBCSuite extends SharedSparkSession {
895896

896897
test("SPARK-53454: AlwaysTrue/AlwaysFalse compile to portable SQL in JDBCSQLBuilder") {
897898
val dialect = JdbcDialects.get("jdbc:")
898-
assert(dialect.compileExpression(new AlwaysTrue).get === "1 = 1")
899-
assert(dialect.compileExpression(new AlwaysFalse).get === "1 = 0")
899+
assert(dialect.compileExpression(new AlwaysTrue).get === "(1 = 1)")
900+
assert(dialect.compileExpression(new AlwaysFalse).get === "(1 = 0)")
901+
902+
// The result must stay valid when AlwaysTrue/AlwaysFalse is nested as an operand
903+
// of a larger expression, not just as a standalone WHERE predicate. Without the
904+
// surrounding parentheses the bare `1 = 1` would inline into invalid SQL such as
905+
// `a = 1 = 1`.
906+
val ref = FieldReference("a")
907+
val eqTrue = new Predicate("=", Array[V2Expression](ref, new AlwaysTrue))
908+
val eqFalse = new Predicate("=", Array[V2Expression](ref, new AlwaysFalse))
909+
assert(dialect.compileExpression(eqTrue).get === "\"a\" = (1 = 1)")
910+
assert(dialect.compileExpression(eqFalse).get === "\"a\" = (1 = 0)")
900911
}
901912

902913
test("Dialect unregister") {

0 commit comments

Comments
 (0)