This is an automated email from the ASF dual-hosted git repository.
dtenedor pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 1adffbc591cd [SPARK-59016][SQL] Expand CHAR/VARCHAR leak inventory to
nested shapes
1adffbc591cd is described below
commit 1adffbc591cdbdcd53ef656689a6895bd15e7e05
Author: Serge Rielau <[email protected]>
AuthorDate: Wed Aug 26 10:36:30 2026 -0700
[SPARK-59016][SQL] Expand CHAR/VARCHAR leak inventory to nested shapes
### What changes were proposed in this pull request?
[SPARK-59016](https://issues.apache.org/jira/browse/SPARK-59016) expands
the FunctionRegistry CHAR/VARCHAR leak inventory to cover additional arities
and nested array, map, and struct shapes.
The allowlist is shape-aware: legitimate collection pass-through calls are
distinguished from scalar transforming calls such as `reverse(c)` and
`concat(c, ...)`, which must return plain STRING. The original scalar inventory
remains under parent
[SPARK-58794](https://issues.apache.org/jira/browse/SPARK-58794), while the
expanded templates are covered by SPARK-59016.
### Why are the changes needed?
The previous inventory used only seven argument templates, so a
CHAR/VARCHAR leak at another arity or nested shape would not fail CI. A
function-name-only allowlist could also hide an invalid scalar leak when
another shape of the same function is a legitimate pass-through.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- `build/sbt "sql/testOnly org.apache.spark.sql.BasicCharVarcharTestSuite
-- -z inventoried"`
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor Grok 4.6
Closes #58299 from srielau/serge-rielau_data/SPARK-58794-p1.
Authored-by: Serge Rielau <[email protected]>
Signed-off-by: Daniel Tenedorio <[email protected]>
(cherry picked from commit 9f99c395ef1a8c89f884e63e072fa0111dccecd0)
Signed-off-by: Daniel Tenedorio <[email protected]>
---
.../spark/sql/catalyst/optimizer/expressions.scala | 2 +
.../apache/spark/sql/CharVarcharTestSuite.scala | 82 +++++++++++++++-------
2 files changed, 57 insertions(+), 27 deletions(-)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
index 580c92f6eed5..7e8e818e37b8 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
@@ -1158,6 +1158,8 @@ object FoldablePropagation extends Rule[LogicalPlan] {
object SimplifyCasts extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan =
plan.transformAllExpressionsWithPruning(
_.containsPattern(CAST), ruleId) {
+ // Annotated STRING (flag off) is not unconstrained STRING. Dropping
CAST(... AS STRING)
+ // would hide the type change. First-class CHAR/VARCHAR already fail
e.dataType == StringType.
case c @ Cast(e: NamedExpression, StringType, _, _)
if e.dataType == StringType &&
e.metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY) => c
case Cast(e, dataType, _, _) if e.dataType == dataType => e
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
index a9e3c0626e0f..7de934ef2af6 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
@@ -1243,43 +1243,71 @@ class BasicCharVarcharTestSuite extends
SharedSparkSession {
}
}
- // Allowlist for the inventory below: pass-through and container cases that
may keep
- // CHAR(n)/VARCHAR(n): aggregates/ordering that return an input unchanged,
null-handling,
- // element access, array/map/struct constructors, and collection
rearrangements that keep
- // element types. Coverage is limited to the seven fixed argumentShapes
templates in the test;
- // a leak only at another arity or nested shape would not fail here. For
those shapes,
- // anything not listed must reduce to plain STRING.
+ // Pass-through and container functions that may keep CHAR(n)/VARCHAR(n):
aggregates
+ // and ordering that return an input unchanged, null-handling, element
access,
+ // array/map/struct constructors, and collection rearrangements that keep
element types.
+ // Legitimacy is still per shape: reverse(array(c)) may keep CHAR,
reverse(c) must not.
private val charVarcharPassThroughFunctions = Set(
"any_value", "approx_top_k", "approx_top_k_accumulate", "array",
"array_agg", "array_compact",
"array_distinct", "array_max", "array_min", "array_repeat", "array_sort",
"arrays_zip",
"coalesce", "collect_list", "collect_set", "collect_union", "concat",
"explode",
- "explode_outer", "first", "first_value", "get", "greatest", "ifnull",
"last", "last_value",
- "least", "map", "max", "max_by", "measure", "min", "min_by", "mode",
"named_struct", "nullif",
- "nullifzero", "nvl", "reverse", "shuffle", "sort_array", "struct",
"trim_array", "when")
+ "explode_outer", "first", "first_value", "flatten", "get", "greatest",
"ifnull", "last",
+ "last_value", "least", "map", "map_concat", "map_entries", "map_keys",
"map_values", "max",
+ "max_by", "measure", "min", "min_by", "mode", "named_struct", "nullif",
"nullifzero", "nvl",
+ "nvl2", "reverse", "shuffle", "sort_array", "struct", "trim_array", "when")
+
+ // String-transforming shapes of otherwise pass-through functions. These
must reduce to
+ // unconstrained STRING even though the same function keeps CHAR on
collection inputs.
+ private val charVarcharTransformingCalls = Set(
+ "concat(c)", "concat(c, c)", "concat(c, c, c)", "concat(c, 'x')",
"concat('x', c)",
+ "reverse(c)")
+
+ private val inventoryScalarShapes = Seq(
+ "%s(c)", "%s(c, c)", "%s(c, 'x')", "%s('x', c)", "%s(c, 1)",
"%s(array(c))",
+ "%s(array(c), '-')")
+
+ private val inventoryNestedShapes = Seq(
+ "%s(c, c, c)", "%s(array(array(c)))", "%s(named_struct('x', c))",
+ "%s(map(c, 1))", "%s(map(1, c))")
+
+ private def assertNoInventoriedCharVarcharLeaks(argumentShapes:
Seq[String]): Unit = {
+ val (passThroughLeaks, transformingLeaks) =
+ FunctionRegistry.functionSet.map(_.funcName).toSeq.sorted.flatMap { name
=>
+ argumentShapes.map(_.format(name)).flatMap { call =>
+ // Most shapes do not typecheck for a given function; those are
simply not evidence.
+ val keepsCharVarchar =
+ Try(sql(s"SELECT $call AS r FROM
std_inventory").schema.head.dataType)
+ .toOption
+ .exists(CharVarcharUtils.hasCharVarchar)
+ Option.when(keepsCharVarchar &&
+ (!charVarcharPassThroughFunctions.contains(name) ||
+ charVarcharTransformingCalls.contains(call)))((name, call))
+ }
+ }.partition { case (_, call) =>
!charVarcharTransformingCalls.contains(call) }
- test("SPARK-58794: inventoried shapes do not leak CHAR/VARCHAR under
standardSemantics") {
- val argumentShapes = Seq(
- "%s(c)", "%s(c, c)", "%s(c, 'x')", "%s('x', c)", "%s(c, 1)",
"%s(array(c))",
- "%s(array(c), '-')")
+ assert(passThroughLeaks.isEmpty,
+ "these inventoried calls returned a CHAR/VARCHAR type; if they
legitimately pass through " +
+ "their input type, add the function name to
charVarcharPassThroughFunctions: " +
+ passThroughLeaks.map(_._2).mkString(", "))
+ assert(transformingLeaks.isEmpty,
+ "these transforming calls returned a CHAR/VARCHAR type; fix the
expression to return " +
+ "plain STRING: " + transformingLeaks.map(_._2).mkString(", "))
+ }
+ test("SPARK-58794: inventoried shapes do not leak CHAR/VARCHAR under
standardSemantics") {
withTable("std_inventory") {
sql("CREATE TABLE std_inventory (c CHAR(5)) USING parquet")
withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
- val leaks = FunctionRegistry.functionSet.map(_.funcName).toSeq.sorted
- .filterNot(charVarcharPassThroughFunctions.contains)
- .flatMap { name =>
- argumentShapes.map(_.format(name)).filter { call =>
- // Most shapes do not typecheck for a given function; those are
simply not evidence.
- Try(sql(s"SELECT $call AS r FROM
std_inventory").schema.head.dataType)
- .toOption
- .exists(CharVarcharUtils.hasCharVarchar)
- }
- }
+ assertNoInventoriedCharVarcharLeaks(inventoryScalarShapes)
+ }
+ }
+ }
- assert(leaks.isEmpty,
- "these inventoried calls returned a CHAR/VARCHAR type; either fix
the expression to " +
- "return plain STRING or add it to charVarcharPassThroughFunctions:
" +
- leaks.mkString(", "))
+ test("SPARK-59016: nested inventoried shapes do not leak CHAR/VARCHAR") {
+ withTable("std_inventory") {
+ sql("CREATE TABLE std_inventory (c CHAR(5)) USING parquet")
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assertNoInventoriedCharVarcharLeaks(inventoryNestedShapes)
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
