This is an automated email from the ASF dual-hosted git repository.

szehon-ho pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.3 by this push:
     new 1308d4c3b887 [SPARK-58782][SQL] Fix DSv2 pushdown null literal 
serialization bug
1308d4c3b887 is described below

commit 1308d4c3b887bc1b897c90b0a75e978a8d68aac4
Author: Pratham Manja <[email protected]>
AuthorDate: Tue Aug 18 17:57:05 2026 -0700

    [SPARK-58782][SQL] Fix DSv2 pushdown null literal serialization bug
    
    ### What changes were proposed in this pull request?
    
    This PR fixes a bug where null literals were incorrectly serialized as the 
string 'null' instead of the SQL keyword NULL when pushed down to external 
databases via DSv2 (JDBC aggregate/predicate pushdown).
    
    ### Why are the changes needed?
    
    `LiteralValue.toString` - the only toString for DSv2's only concrete 
Literal, and the path every pushdown rendering (explain strings, generated SQL) 
ultimately goes through,  matched on dataType even when the value was null, 
instead of checking for null first. This produced two distinct, user-visible 
bugs:
    
    * For StringType, a null value rendered as the quoted string 'null' instead 
of NULL. When pushed down (e.g. NULLIF(s, ''), which rewrites to If(s = '', 
null, s)), the empty-string row was grouped under the literal string "null" 
instead of collapsing into the real NULL group — the external database executed 
a different grouping key than intended, producing 3 result rows instead of 2.
    
    * For BinaryType, the branch ran assert(value.isInstanceOf[Array[Byte]]), 
which fails on null and threw an AssertionError when rendering a pushed 
expression containing a null binary literal, rather than merely producing bad 
SQL.
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes - Query results will now be correct when null literals are pushed down 
to external databases via JDBC.
    
    _Before the fix:_ A query with NULLIF(col, '') in a GROUP BY would push 
down CASE WHEN col = '' THEN 'null' ELSE col END to the database, creating a 
spurious 'null' string group instead of a NULL group.
    
    _After the fix:_ The same query pushes down CASE WHEN col = '' THEN NULL 
ELSE col END, correctly collapsing empty strings into the NULL group as 
intended. Null binary literals render as NULL without error.
    
    ### How was this patch tested?
    
    * Unit tests in LiteralValueSuite covering all data types
    * End-to-end regression test in JDBCV2Suite with H2 database
    * Updated existing test expectations that relied on the old (incorrect) 
behavior
    ### Was this patch authored or co-authored using generative AI tooling?
    
    No
    
    Closes #58023 from pratham76/SPARK-58782.
    
    Authored-by: Pratham Manja <[email protected]>
    Signed-off-by: Szehon Ho <[email protected]>
    (cherry picked from commit 01b0663a711f9ca30f79b669841132bf83e1942d)
    Signed-off-by: Szehon Ho <[email protected]>
---
 .../sql/connector/expressions/expressions.scala    | 20 ++++++---
 .../connector/expressions/LiteralValueSuite.scala  | 52 ++++++++++++++++++++++
 .../org/apache/spark/sql/jdbc/JDBCV2Suite.scala    | 26 +++++++++--
 3 files changed, 88 insertions(+), 10 deletions(-)

diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/expressions/expressions.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/expressions/expressions.scala
index 18d94969aa27..0028f78d22f9 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/expressions/expressions.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/expressions/expressions.scala
@@ -388,13 +388,19 @@ private[sql] object HoursTransform {
 }
 
 private[sql] final case class LiteralValue[T](value: T, dataType: DataType) 
extends Literal[T] {
-  override def toString: String = dataType match {
-    case StringType => s"'${s"$value".replace("'", "''")}'"
-    case BinaryType =>
-      assert(value.isInstanceOf[Array[Byte]])
-      val bytes = value.asInstanceOf[Array[Byte]]
-      "0x" + HexFormat.of().withUpperCase().formatHex(bytes)
-    case _ => s"$value"
+  override def toString: String = {
+    if (value == null) {
+      "NULL"
+    } else {
+      dataType match {
+        case StringType => s"'${s"$value".replace("'", "''")}'"
+        case BinaryType =>
+          assert(value.isInstanceOf[Array[Byte]])
+          val bytes = value.asInstanceOf[Array[Byte]]
+          "0x" + HexFormat.of().withUpperCase().formatHex(bytes)
+        case _ => s"$value"
+      }
+    }
   }
 }
 
diff --git 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/expressions/LiteralValueSuite.scala
 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/expressions/LiteralValueSuite.scala
new file mode 100644
index 000000000000..9a21f14e022a
--- /dev/null
+++ 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/expressions/LiteralValueSuite.scala
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connector.expressions
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types.{BinaryType, BooleanType, IntegerType, 
StringType}
+
+class LiteralValueSuite extends SparkFunSuite {
+
+  test("SPARK-58782: null literals should render as NULL") {
+    assert(LiteralValue(null, StringType).toString === "NULL")
+    assert(LiteralValue(null, IntegerType).toString === "NULL")
+    assert(LiteralValue(null, BooleanType).toString === "NULL")
+    assert(LiteralValue(null, BinaryType).toString === "NULL")
+  }
+
+  test("non-null string literals should be quoted") {
+    assert(LiteralValue("test", StringType).toString === "'test'")
+    assert(LiteralValue("", StringType).toString === "''")
+    assert(LiteralValue("it's", StringType).toString === "'it''s'")
+  }
+
+  test("non-null numeric literals should not be quoted") {
+    assert(LiteralValue(42, IntegerType).toString === "42")
+    assert(LiteralValue(0, IntegerType).toString === "0")
+  }
+
+  test("non-null boolean literals should not be quoted") {
+    assert(LiteralValue(true, BooleanType).toString === "true")
+    assert(LiteralValue(false, BooleanType).toString === "false")
+  }
+
+  test("non-null binary literals should render as hex") {
+    val bytes = Array[Byte](0x12, 0x34, 0xAB.toByte, 0xCD.toByte)
+    assert(LiteralValue(bytes, BinaryType).toString === "0x1234ABCD")
+  }
+}
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 d7e5786ecb9c..8f456c3e57ea 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
@@ -236,6 +236,12 @@ class JDBCV2Suite extends SharedSparkSession with 
ExplainSuiteHelper {
       batchStmt.addBatch("INSERT INTO \"test\".\"strings_with_nulls\" VALUES 
('a a a')")
       batchStmt.addBatch("INSERT INTO \"test\".\"strings_with_nulls\" VALUES 
(null)")
 
+      batchStmt.addBatch(
+        "CREATE TABLE \"test\".\"null_literal\" (s TEXT(32))")
+      batchStmt.addBatch("INSERT INTO \"test\".\"null_literal\" VALUES 
('keep')")
+      batchStmt.addBatch("INSERT INTO \"test\".\"null_literal\" VALUES ('')")
+      batchStmt.addBatch("INSERT INTO \"test\".\"null_literal\" VALUES (null)")
+
       batchStmt.executeBatch()
 
       conn
@@ -1819,7 +1825,8 @@ class JDBCV2Suite extends SharedSparkSession with 
ExplainSuiteHelper {
       Seq(Row("test", "address", false), Row("test", "people", false),
         Row("test", "empty_table", false), Row("test", "employee", false),
         Row("test", "item", false), Row("test", "dept", false),
-        Row("test", "person", false), Row("test", "view1", false), Row("test", 
"view2", false),
+        Row("test", "null_literal", false), Row("test", "person", false),
+        Row("test", "view1", false), Row("test", "view2", false),
         Row("test", "datetime", false), Row("test", "binary_tab", false),
         Row("test", "employee_bonus", false),
         Row("test", "strings_with_nulls", false)))
@@ -2515,7 +2522,7 @@ class JDBCV2Suite extends SharedSparkSession with 
ExplainSuiteHelper {
     checkAggregateRemoved(df3)
     checkPushedInfo(df3,
       """
-        |PushedAggregates: [AVG(CASE WHEN BONUS IS NOT NULL THEN BONUS ELSE 
null END)],
+        |PushedAggregates: [AVG(CASE WHEN BONUS IS NOT NULL THEN BONUS ELSE 
NULL END)],
         |PushedFilters: [DEPT IS NOT NULL, DEPT > 0],
         |PushedGroupByExpressions: [DEPT],
         |""".stripMargin.replaceAll("\n", " "))
@@ -2531,7 +2538,7 @@ class JDBCV2Suite extends SharedSparkSession with 
ExplainSuiteHelper {
     checkAggregateRemoved(df4)
     checkPushedInfo(df4,
       """
-        |PushedAggregates: [AVG(DISTINCT CASE WHEN BONUS IS NOT NULL THEN 
BONUS ELSE null END)],
+        |PushedAggregates: [AVG(DISTINCT CASE WHEN BONUS IS NOT NULL THEN 
BONUS ELSE NULL END)],
         |PushedFilters: [DEPT IS NOT NULL, DEPT > 0],
         |PushedGroupByExpressions: [DEPT],
         |""".stripMargin.replaceAll("\n", " "))
@@ -3174,4 +3181,17 @@ class JDBCV2Suite extends SharedSparkSession with 
ExplainSuiteHelper {
 
     assertResult(expectedMetadata) { jdbcRdd.getDatabaseMetadata }
   }
+
+  test("SPARK-58782: null literal in aggregate should render as NULL not 
'null'") {
+    val df = sql("SELECT NULLIF(s, '') AS g, COUNT(*) FROM 
h2.test.null_literal GROUP BY g")
+
+    checkAggregateRemoved(df)
+    checkPushedInfo(df,
+      "PushedAggregates: [COUNT(*)]",
+      "PushedGroupByExpressions: [CASE WHEN S = '' THEN NULL ELSE S END]")
+
+    // The '' row should collapse into the NULL group, not create a separate 
'null' string group
+    checkAnswer(df, Seq(Row("keep", 1), Row(null, 2)))
+  }
+
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to