From 287946eb6144aafed32a9c924be8a776f0f3fe19 Mon Sep 17 00:00:00 2001 From: srielau Date: Wed, 26 Aug 2026 01:13:53 +0000 Subject: [PATCH 1/3] [SPARK-59016][SQL] Keep CHAR/VARCHAR through SimplifyCasts, V2 prune, and inventory CAST to unconstrained STRING must not be dropped for first-class CHAR/VARCHAR or annotated STRING. V2 scan output keeps those types under standardSemantics. Widen the FunctionRegistry leak inventory and allowlist true pass-throughs. --- .../sql/catalyst/optimizer/expressions.scala | 6 ++++ .../optimizer/SimplifyCastsSuite.scala | 26 ++++++++++++++ .../datasources/v2/PushDownUtils.scala | 8 +++-- .../spark/sql/CharVarcharTestSuite.scala | 34 ++++++++++++++----- 4 files changed, 63 insertions(+), 11 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 580c92f6eed5a..23803298042a6 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 @@ -31,6 +31,7 @@ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules._ import org.apache.spark.sql.catalyst.trees.{AlwaysProcess, TreeNodeTag} import org.apache.spark.sql.catalyst.trees.TreePattern._ +import org.apache.spark.sql.catalyst.util.CharVarcharUtils import org.apache.spark.sql.catalyst.util.CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY import org.apache.spark.sql.catalyst.util.UnsafeRowUtils.isBinaryStable import org.apache.spark.sql.internal.SQLConf @@ -1158,8 +1159,13 @@ object FoldablePropagation extends Rule[LogicalPlan] { object SimplifyCasts extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan.transformAllExpressionsWithPruning( _.containsPattern(CAST), ruleId) { + // Annotated STRING (flag off) and first-class CHAR/VARCHAR are not unconstrained STRING. + // Dropping CAST(... AS STRING) would hide the type change. case c @ Cast(e: NamedExpression, StringType, _, _) if e.dataType == StringType && e.metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY) => c + case c @ Cast(e, dt: StringType, _, _) + if CharVarcharUtils.hasCharVarchar(e.dataType) && + !CharVarcharUtils.hasCharVarchar(dt) => c case Cast(e, dataType, _, _) if e.dataType == dataType => e case c @ Cast(Cast(e, dt1: NumericType, _, _), dt2: NumericType, _, _) if isWiderCast(e.dataType, dt1) && isWiderCast(dt1, dt2) => diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala index 2eb4830bca98b..29179f919be70 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala @@ -19,9 +19,11 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ +import org.apache.spark.sql.catalyst.expressions.{Alias, AttributeReference, Cast} import org.apache.spark.sql.catalyst.plans.PlanTest import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules.RuleExecutor +import org.apache.spark.sql.catalyst.util.CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types._ @@ -138,4 +140,28 @@ class SimplifyCastsSuite extends PlanTest { input.select($"a".cast(DecimalType(2, 1)).as("v")).analyze), input.select($"a".cast(DecimalType(2, 1)).as("v")).analyze) } + + test("SPARK-59016: do not drop CAST from CHAR/VARCHAR or annotated STRING to STRING") { + def keepsCast(plan: LogicalPlan): Boolean = + plan.exists(_.expressions.exists(_.exists(_.isInstanceOf[Cast]))) + + val charCol = AttributeReference("c", CharType(5))() + val charPlan = Project(Seq(Alias(Cast(charCol, StringType), "s")()), LocalRelation(charCol)) + val charOpt = Optimize.execute(charPlan) + assert(keepsCast(charOpt), charOpt) + + val varcharCol = AttributeReference("v", VarcharType(5))() + val varcharPlan = + Project(Seq(Alias(Cast(varcharCol, StringType), "s")()), LocalRelation(varcharCol)) + val varcharOpt = Optimize.execute(varcharPlan) + assert(keepsCast(varcharOpt), varcharOpt) + + val metadata = new MetadataBuilder() + .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, "char(5)").build() + val annotated = AttributeReference("c", StringType, metadata = metadata)() + val annotatedPlan = + Project(Seq(Alias(Cast(annotated, StringType), "s")()), LocalRelation(annotated)) + val annotatedOpt = Optimize.execute(annotatedPlan) + assert(keepsCast(annotatedOpt), annotatedOpt) + } } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala index fa330212370fb..e1f66e27b8c93 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala @@ -704,8 +704,12 @@ object PushDownUtils extends Logging { schema: StructType, relation: DataSourceV2Relation): Seq[AttributeReference] = { val nameToAttr = Utils.toMap(relation.output.map(_.name), relation.output) - val cleaned = CharVarcharUtils.replaceCharVarcharWithStringInSchema(schema) - toAttributes(cleaned).map { + // Under standardSemantics the scan keeps first-class CHAR/VARCHAR. Flag-off still + // rewrites to annotated STRING so ApplyCharTypePadding can find the original type. + val outputSchema = + if (SQLConf.get.charVarcharStandardSemantics) schema + else CharVarcharUtils.replaceCharVarcharWithStringInSchema(schema) + toAttributes(outputSchema).map { // we have to keep the attribute id during transformation a => a.withExprId(nameToAttr(a.name).exprId) } 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 a9e3c0626e0fd..0443d62524fd2 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 @@ -1246,21 +1246,22 @@ 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. + // element types. For the listed argument shapes, anything not listed must reduce to plain + // STRING. 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") - test("SPARK-58794: inventoried shapes do not leak CHAR/VARCHAR under standardSemantics") { + test("SPARK-59016: 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), '-')") + "%s(c)", "%s(c, c)", "%s(c, c, c)", "%s(c, 'x')", "%s('x', c)", "%s(c, 1)", + "%s(array(c))", "%s(array(c), '-')", "%s(array(array(c)))", + "%s(named_struct('x', c))", "%s(map(c, 1))", "%s(map(1, c))") withTable("std_inventory") { sql("CREATE TABLE std_inventory (c CHAR(5)) USING parquet") @@ -2423,6 +2424,21 @@ class DSV2CharVarcharTestSuite extends CharVarcharTestSuite } } + test("SPARK-59016: V2 column prune keeps CHAR/VARCHAR under standardSemantics") { + withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") { + withTable("std_v2_prune") { + sql(s"CREATE TABLE std_v2_prune (c CHAR(5), v VARCHAR(5), i INT) USING $format") + sql("INSERT INTO std_v2_prune VALUES ('ab', 'cd', 1)") + val charDf = sql("SELECT c FROM std_v2_prune WHERE c = 'ab '") + assert(charDf.schema.head.dataType === CharType(5)) + checkAnswer(charDf, Row("ab ")) + val varcharDf = sql("SELECT v FROM std_v2_prune WHERE v = 'cd'") + assert(varcharDf.schema.head.dataType === VarcharType(5)) + checkAnswer(varcharDf, Row("cd")) + } + } + } + test("char/varchar type values length check: partitioned columns of other types") { Seq("CHAR(5)", "VARCHAR(5)").foreach { typ => withTable("t") { From 085a7fca688ddfa7d1df58cf0a683fbd580b84fc Mon Sep 17 00:00:00 2001 From: srielau Date: Wed, 26 Aug 2026 14:13:39 +0000 Subject: [PATCH 2/3] [SPARK-59016][SQL] Drop redundant SimplifyCasts/V2 prune CHAR special-cases Keep CHAR/VARCHAR inventory coverage; make the allowlist shape-aware so string reverse/concat cannot hide behind collection pass-throughs. --- .../sql/catalyst/optimizer/expressions.scala | 8 +- .../optimizer/SimplifyCastsSuite.scala | 26 ------ .../datasources/v2/PushDownUtils.scala | 8 +- .../spark/sql/CharVarcharTestSuite.scala | 85 +++++++++++-------- 4 files changed, 52 insertions(+), 75 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 23803298042a6..7e8e818e37b8a 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 @@ -31,7 +31,6 @@ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules._ import org.apache.spark.sql.catalyst.trees.{AlwaysProcess, TreeNodeTag} import org.apache.spark.sql.catalyst.trees.TreePattern._ -import org.apache.spark.sql.catalyst.util.CharVarcharUtils import org.apache.spark.sql.catalyst.util.CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY import org.apache.spark.sql.catalyst.util.UnsafeRowUtils.isBinaryStable import org.apache.spark.sql.internal.SQLConf @@ -1159,13 +1158,10 @@ object FoldablePropagation extends Rule[LogicalPlan] { object SimplifyCasts extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan.transformAllExpressionsWithPruning( _.containsPattern(CAST), ruleId) { - // Annotated STRING (flag off) and first-class CHAR/VARCHAR are not unconstrained STRING. - // Dropping CAST(... AS STRING) would hide the type change. + // 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 c @ Cast(e, dt: StringType, _, _) - if CharVarcharUtils.hasCharVarchar(e.dataType) && - !CharVarcharUtils.hasCharVarchar(dt) => c case Cast(e, dataType, _, _) if e.dataType == dataType => e case c @ Cast(Cast(e, dt1: NumericType, _, _), dt2: NumericType, _, _) if isWiderCast(e.dataType, dt1) && isWiderCast(dt1, dt2) => diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala index 29179f919be70..2eb4830bca98b 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala @@ -19,11 +19,9 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ -import org.apache.spark.sql.catalyst.expressions.{Alias, AttributeReference, Cast} import org.apache.spark.sql.catalyst.plans.PlanTest import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules.RuleExecutor -import org.apache.spark.sql.catalyst.util.CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types._ @@ -140,28 +138,4 @@ class SimplifyCastsSuite extends PlanTest { input.select($"a".cast(DecimalType(2, 1)).as("v")).analyze), input.select($"a".cast(DecimalType(2, 1)).as("v")).analyze) } - - test("SPARK-59016: do not drop CAST from CHAR/VARCHAR or annotated STRING to STRING") { - def keepsCast(plan: LogicalPlan): Boolean = - plan.exists(_.expressions.exists(_.exists(_.isInstanceOf[Cast]))) - - val charCol = AttributeReference("c", CharType(5))() - val charPlan = Project(Seq(Alias(Cast(charCol, StringType), "s")()), LocalRelation(charCol)) - val charOpt = Optimize.execute(charPlan) - assert(keepsCast(charOpt), charOpt) - - val varcharCol = AttributeReference("v", VarcharType(5))() - val varcharPlan = - Project(Seq(Alias(Cast(varcharCol, StringType), "s")()), LocalRelation(varcharCol)) - val varcharOpt = Optimize.execute(varcharPlan) - assert(keepsCast(varcharOpt), varcharOpt) - - val metadata = new MetadataBuilder() - .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, "char(5)").build() - val annotated = AttributeReference("c", StringType, metadata = metadata)() - val annotatedPlan = - Project(Seq(Alias(Cast(annotated, StringType), "s")()), LocalRelation(annotated)) - val annotatedOpt = Optimize.execute(annotatedPlan) - assert(keepsCast(annotatedOpt), annotatedOpt) - } } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala index e1f66e27b8c93..fa330212370fb 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala @@ -704,12 +704,8 @@ object PushDownUtils extends Logging { schema: StructType, relation: DataSourceV2Relation): Seq[AttributeReference] = { val nameToAttr = Utils.toMap(relation.output.map(_.name), relation.output) - // Under standardSemantics the scan keeps first-class CHAR/VARCHAR. Flag-off still - // rewrites to annotated STRING so ApplyCharTypePadding can find the original type. - val outputSchema = - if (SQLConf.get.charVarcharStandardSemantics) schema - else CharVarcharUtils.replaceCharVarcharWithStringInSchema(schema) - toAttributes(outputSchema).map { + val cleaned = CharVarcharUtils.replaceCharVarcharWithStringInSchema(schema) + toAttributes(cleaned).map { // we have to keep the attribute id during transformation a => a.withExprId(nameToAttr(a.name).exprId) } 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 0443d62524fd2..40b6cb3b2b9c1 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,11 +1243,10 @@ 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. For the listed argument 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", @@ -1257,30 +1256,57 @@ class BasicCharVarcharTestSuite extends SharedSparkSession { "max_by", "measure", "min", "min_by", "mode", "named_struct", "nullif", "nullifzero", "nvl", "nvl2", "reverse", "shuffle", "sort_array", "struct", "trim_array", "when") - test("SPARK-59016: inventoried shapes do not leak CHAR/VARCHAR under standardSemantics") { - val argumentShapes = Seq( - "%s(c)", "%s(c, c)", "%s(c, c, c)", "%s(c, 'x')", "%s('x', c)", "%s(c, 1)", - "%s(array(c))", "%s(array(c), '-')", "%s(array(array(c)))", - "%s(named_struct('x', c))", "%s(map(c, 1))", "%s(map(1, c))") + // 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 inventoriedCharVarcharLeaks(argumentShapes: Seq[String]): Seq[String] = { + FunctionRegistry.functionSet.map(_.funcName).toSeq.sorted.flatMap { name => + argumentShapes.map(_.format(name)).filter { 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) + keepsCharVarchar && + (!charVarcharPassThroughFunctions.contains(name) || + charVarcharTransformingCalls.contains(call)) + } + } + } + + 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) - } - } + val leaks = inventoriedCharVarcharLeaks(inventoryScalarShapes) + assert(leaks.isEmpty, + "these inventoried calls returned a CHAR/VARCHAR type; either fix the expression to " + + "return plain STRING, add the function to charVarcharPassThroughFunctions, or " + + "remove it from charVarcharTransformingCalls: " + 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") { + val leaks = inventoriedCharVarcharLeaks(inventoryNestedShapes) 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(", ")) + "return plain STRING, add the function to charVarcharPassThroughFunctions, or " + + "remove it from charVarcharTransformingCalls: " + leaks.mkString(", ")) } } } @@ -2424,21 +2450,6 @@ class DSV2CharVarcharTestSuite extends CharVarcharTestSuite } } - test("SPARK-59016: V2 column prune keeps CHAR/VARCHAR under standardSemantics") { - withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") { - withTable("std_v2_prune") { - sql(s"CREATE TABLE std_v2_prune (c CHAR(5), v VARCHAR(5), i INT) USING $format") - sql("INSERT INTO std_v2_prune VALUES ('ab', 'cd', 1)") - val charDf = sql("SELECT c FROM std_v2_prune WHERE c = 'ab '") - assert(charDf.schema.head.dataType === CharType(5)) - checkAnswer(charDf, Row("ab ")) - val varcharDf = sql("SELECT v FROM std_v2_prune WHERE v = 'cd'") - assert(varcharDf.schema.head.dataType === VarcharType(5)) - checkAnswer(varcharDf, Row("cd")) - } - } - } - test("char/varchar type values length check: partitioned columns of other types") { Seq("CHAR(5)", "VARCHAR(5)").foreach { typ => withTable("t") { From 0b1f49576a158d51ea8831ec5acd0f45017c9dec Mon Sep 17 00:00:00 2001 From: srielau Date: Wed, 26 Aug 2026 14:35:37 +0000 Subject: [PATCH 3/3] [SPARK-59016][SQL][FOLLOWUP] Clarify CHAR/VARCHAR inventory failures Separate legitimate pass-through exemptions from transforming calls that must be fixed. --- .../spark/sql/CharVarcharTestSuite.scala | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) 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 40b6cb3b2b9c1..7de934ef2af6f 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 @@ -1270,30 +1270,35 @@ class BasicCharVarcharTestSuite extends SharedSparkSession { "%s(c, c, c)", "%s(array(array(c)))", "%s(named_struct('x', c))", "%s(map(c, 1))", "%s(map(1, c))") - private def inventoriedCharVarcharLeaks(argumentShapes: Seq[String]): Seq[String] = { - FunctionRegistry.functionSet.map(_.funcName).toSeq.sorted.flatMap { name => - argumentShapes.map(_.format(name)).filter { 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) - keepsCharVarchar && - (!charVarcharPassThroughFunctions.contains(name) || - charVarcharTransformingCalls.contains(call)) - } - } + 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) } + + 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 = inventoriedCharVarcharLeaks(inventoryScalarShapes) - assert(leaks.isEmpty, - "these inventoried calls returned a CHAR/VARCHAR type; either fix the expression to " + - "return plain STRING, add the function to charVarcharPassThroughFunctions, or " + - "remove it from charVarcharTransformingCalls: " + leaks.mkString(", ")) + assertNoInventoriedCharVarcharLeaks(inventoryScalarShapes) } } } @@ -1302,11 +1307,7 @@ class BasicCharVarcharTestSuite extends SharedSparkSession { withTable("std_inventory") { sql("CREATE TABLE std_inventory (c CHAR(5)) USING parquet") withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") { - val leaks = inventoriedCharVarcharLeaks(inventoryNestedShapes) - assert(leaks.isEmpty, - "these inventoried calls returned a CHAR/VARCHAR type; either fix the expression to " + - "return plain STRING, add the function to charVarcharPassThroughFunctions, or " + - "remove it from charVarcharTransformingCalls: " + leaks.mkString(", ")) + assertNoInventoriedCharVarcharLeaks(inventoryNestedShapes) } } }