Skip to content

[SPARK-53454][SQL][FOLLOWUP] Parenthesize AlwaysTrue/AlwaysFalse SQL so it is valid as a nested operand #56263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cloud-fan wants to merge 2 commits into from
Closed

[SPARK-53454][SQL][FOLLOWUP] Parenthesize AlwaysTrue/AlwaysFalse SQL so it is valid as a nested operand #56263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf60a78
[SPARK-53454][SQL][FOLLOWUP] Parenthesize AlwaysTrue/AlwaysFalse SQL …
cloud-fan Jun 2, 2026
39ef38f
Address review: drop weird backtick pair in comment
cloud-fan Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
17 changes: 14 additions & 3 deletions sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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") {
Expand Down