srielau commented on code in PR #58299:
URL: https://github.com/apache/spark/pull/58299#discussion_r3863682392
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyCastsSuite.scala:
##########
@@ -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])))
Review Comment:
Removed the test with the production arm. `keepsCast` would have passed
without the change, and `comparePlans` would not have moved either.
##########
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") {
Review Comment:
Restored SPARK-58794 for the original scalar shapes and added SPARK-59016
for the nested/map/struct templates. The function allowlist is now paired with
`charVarcharTransformingCalls` so string `reverse(c)` / `concat(c, ...)` cannot
hide behind collection pass-throughs of the same function.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -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))
Review Comment:
Dropped the prune test with the `toOutputAttrs` special-case. The type
assertion could pass without that change because first-class types already
survive `replaceCharVarcharWithStringInSchema`. A width/preserve-only/flag-off
matrix would not have been testing a real prune-path bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
