srielau commented on code in PR #58080:
URL: https://github.com/apache/spark/pull/58080#discussion_r3815907215
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCoercion.scala:
##########
@@ -429,8 +432,18 @@ object CollationTypeCoercion extends SQLConfHelper {
(left.strength.priority, right.strength.priority) match {
case (leftPriority, rightPriority) if leftPriority == rightPriority =>
- if (left.sameType(right)) left
- else handleMismatch()
+ if (left.sameType(right)) {
+ left
+ } else {
+ // Equal strength with differing types is only a real collation
mismatch when the
+ // collations differ. Same-collation CHAR(2) and CHAR(4) differ only
in length, so widen
+ // to the string-family LCT (max(n, m), which pads rather than
truncates); a genuine
+ // collation mismatch has no LCT and falls through.
+ StringHelper.tightestCommonString(left.stringType, right.stringType)
match {
+ case Some(lct) => StringTypeWithContext(lct, left.strength)
+ case None => handleMismatch()
+ }
+ }
case (leftPriority, rightPriority) =>
if (leftPriority < rightPriority) left
Review Comment:
Fixed in 64008e1. `getWinningStringType` now takes `tightestCommonString`
(max length) whenever the collations match, and only then attaches the stronger
strength.
The narrowing case that actually shows up in SQL is Implicit vs Default, not
the COLLATE operator: `CAST('a' AS CHAR(2) COLLATE UTF8_LCASE)` is Implicit,
`CAST(1 AS CHAR(4) COLLATE UTF8_LCASE)` is Default, and the old
unequal-strength branch returned CHAR(2). `expr COLLATE name` itself is STRING
(ResolvedCollation), so it cannot carry a CHAR length.
Covered in the golden file and in `BasicCharVarcharTestSuite` under both
`standardSemantics` and `preserveCharVarcharTypeInfo`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCoercion.scala:
##########
@@ -429,8 +432,18 @@ object CollationTypeCoercion extends SQLConfHelper {
(left.strength.priority, right.strength.priority) match {
case (leftPriority, rightPriority) if leftPriority == rightPriority =>
- if (left.sameType(right)) left
- else handleMismatch()
+ if (left.sameType(right)) {
+ left
+ } else {
+ // Equal strength with differing types is only a real collation
mismatch when the
+ // collations differ. Same-collation CHAR(2) and CHAR(4) differ only
in length, so widen
+ // to the string-family LCT (max(n, m), which pads rather than
truncates); a genuine
+ // collation mismatch has no LCT and falls through.
Review Comment:
Done. The method scaladoc now states the strength-vs-length split, that this
rule is not `standardSemantics`-only (`charVarcharFirstClassTypes` is also true
under `preserveCharVarcharTypeInfo`), and includes the Implicit CHAR(2) vs
Default CHAR(4) example.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -1201,6 +1201,108 @@ class BasicCharVarcharTestSuite extends
SharedSparkSession {
}
}
+ test("SPARK-58794: compare, IN and set-ops cast every participant to the
string LCT") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ // Comparison and IN cast both sides (including the IN left-hand side)
to the LCT of all
+ // participants. Casting to CHAR pads, so unequal CHAR lengths compare
equal after widen;
+ // casting to VARCHAR/STRING keeps the CHAR pad, so equality needs a
matching blank.
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a' AS CHAR(4))"),
Row(true))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a' AS
VARCHAR(2))"), Row(false))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a ' AS
VARCHAR(2))"), Row(true))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = 'a'"), Row(false))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = 'a '"), Row(true))
+
+ // INTypeCoercion uses findWiderCommonType over (lhs +: list), then
casts every child:
+ // the left-hand side is not exempt.
+ val inAnalyzed = sql(
+ "SELECT cast('a' AS CHAR(2)) IN (cast('a' AS CHAR(4)), cast('b' AS
VARCHAR(3)))")
+ .queryExecution.analyzed
+ assert(inAnalyzed.toString.contains("as varchar(4)"),
Review Comment:
Agreed on the plan substring. The three-part IN query is now in
`charvarchar-standard-semantics.sql`; the analyzer golden shows the LHS and
both list elements widening to `varchar(4)`. I dropped the duplicated compare /
IN / set-op `checkAnswer`s from the unit suite rather than keeping a second
copy of the golden matrix.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -1201,6 +1201,108 @@ class BasicCharVarcharTestSuite extends
SharedSparkSession {
}
}
+ test("SPARK-58794: compare, IN and set-ops cast every participant to the
string LCT") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ // Comparison and IN cast both sides (including the IN left-hand side)
to the LCT of all
+ // participants. Casting to CHAR pads, so unequal CHAR lengths compare
equal after widen;
+ // casting to VARCHAR/STRING keeps the CHAR pad, so equality needs a
matching blank.
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a' AS CHAR(4))"),
Row(true))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a' AS
VARCHAR(2))"), Row(false))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a ' AS
VARCHAR(2))"), Row(true))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = 'a'"), Row(false))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = 'a '"), Row(true))
+
+ // INTypeCoercion uses findWiderCommonType over (lhs +: list), then
casts every child:
+ // the left-hand side is not exempt.
+ val inAnalyzed = sql(
+ "SELECT cast('a' AS CHAR(2)) IN (cast('a' AS CHAR(4)), cast('b' AS
VARCHAR(3)))")
+ .queryExecution.analyzed
+ assert(inAnalyzed.toString.contains("as varchar(4)"),
+ s"LHS and list should all widen to VARCHAR(4), got:\n$inAnalyzed")
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2)) IN (cast('a' AS CHAR(4)))"),
Row(true))
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2)) IN (cast('a' AS VARCHAR(2)))"),
Row(false))
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2)) IN (cast('a ' AS VARCHAR(2)))"),
Row(true))
+
+ // RTRIM ignores trailing blanks for CHAR vs STRING and for mixed CHAR
lengths.
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2) COLLATE UTF8_BINARY_RTRIM) = 'a'"),
Row(true))
+ checkAnswer(
+ sql("""SELECT cast('a' AS CHAR(2) COLLATE UTF8_BINARY_RTRIM) =
+ | cast('a' AS CHAR(4) COLLATE
UTF8_BINARY_RTRIM)""".stripMargin),
+ Row(true))
+
+ // Collated LCT must keep the collation and take max(n, m), not become
indeterminate.
+ assert(sql(
+ """SELECT coalesce(
+ | cast('a' AS CHAR(2) COLLATE UTF8_LCASE),
+ | cast('bb' AS CHAR(4) COLLATE UTF8_LCASE)) AS c""".stripMargin)
+ .schema.head.dataType === CharType(4, "UTF8_LCASE"))
+
+ // Set ops and multi-row VALUES share the same LCT.
+ assert(sql(
+ """SELECT c FROM (
+ | SELECT cast('a' AS CHAR(2)) AS c UNION SELECT cast('a' AS
CHAR(4)) AS c) t"""
+ .stripMargin)
+ .schema.head.dataType === CharType(4))
+ checkAnswer(
+ sql("""SELECT c FROM (
+ | SELECT cast('a' AS CHAR(2)) AS c UNION SELECT cast('a' AS
CHAR(4)) AS c) t"""
+ .stripMargin),
+ Row("a "))
+ checkAnswer(
+ sql("""SELECT c FROM (
+ | SELECT cast('ab' AS CHAR(2)) AS c INTERSECT
+ | SELECT cast('ab' AS CHAR(4)) AS c) t""".stripMargin),
+ Row("ab "))
+ checkAnswer(
+ sql("""SELECT c FROM (
+ | SELECT cast('ab' AS CHAR(2)) AS c EXCEPT
+ | SELECT cast('ab' AS CHAR(4)) AS c) t""".stripMargin),
+ Seq.empty)
+ assert(sql(
+ """SELECT c FROM (VALUES (cast('a' AS CHAR(2))), (cast('bb' AS
CHAR(4)))) t(c)""")
+ .schema.head.dataType === CharType(4))
+ }
+ }
+
+ test("SPARK-58794: parameterized CHAR/VARCHAR lengths under
standardSemantics") {
Review Comment:
Going to keep this test here rather than move it. Parameterized `CHAR(:n)` /
`VARCHAR(?)` is how a bound length becomes a first-class type; that is the same
CAST / LCT story this PR is pinning, and #58087 is language surfaces
(ORC/Avro/CTAS/VIEW), not a better home.
Negative / non-integral lengths now use `checkError` (`PARSE_SYNTAX_ERROR`
after substitution, `EXCEED_LIMIT_LENGTH` for overflow).
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -1201,6 +1201,108 @@ class BasicCharVarcharTestSuite extends
SharedSparkSession {
}
}
+ test("SPARK-58794: compare, IN and set-ops cast every participant to the
string LCT") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ // Comparison and IN cast both sides (including the IN left-hand side)
to the LCT of all
+ // participants. Casting to CHAR pads, so unequal CHAR lengths compare
equal after widen;
+ // casting to VARCHAR/STRING keeps the CHAR pad, so equality needs a
matching blank.
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a' AS CHAR(4))"),
Row(true))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a' AS
VARCHAR(2))"), Row(false))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = cast('a ' AS
VARCHAR(2))"), Row(true))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = 'a'"), Row(false))
+ checkAnswer(sql("SELECT cast('a' AS CHAR(2)) = 'a '"), Row(true))
+
+ // INTypeCoercion uses findWiderCommonType over (lhs +: list), then
casts every child:
+ // the left-hand side is not exempt.
+ val inAnalyzed = sql(
+ "SELECT cast('a' AS CHAR(2)) IN (cast('a' AS CHAR(4)), cast('b' AS
VARCHAR(3)))")
+ .queryExecution.analyzed
+ assert(inAnalyzed.toString.contains("as varchar(4)"),
+ s"LHS and list should all widen to VARCHAR(4), got:\n$inAnalyzed")
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2)) IN (cast('a' AS CHAR(4)))"),
Row(true))
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2)) IN (cast('a' AS VARCHAR(2)))"),
Row(false))
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2)) IN (cast('a ' AS VARCHAR(2)))"),
Row(true))
+
+ // RTRIM ignores trailing blanks for CHAR vs STRING and for mixed CHAR
lengths.
+ checkAnswer(
+ sql("SELECT cast('a' AS CHAR(2) COLLATE UTF8_BINARY_RTRIM) = 'a'"),
Row(true))
+ checkAnswer(
+ sql("""SELECT cast('a' AS CHAR(2) COLLATE UTF8_BINARY_RTRIM) =
+ | cast('a' AS CHAR(4) COLLATE
UTF8_BINARY_RTRIM)""".stripMargin),
+ Row(true))
+
+ // Collated LCT must keep the collation and take max(n, m), not become
indeterminate.
+ assert(sql(
+ """SELECT coalesce(
+ | cast('a' AS CHAR(2) COLLATE UTF8_LCASE),
+ | cast('bb' AS CHAR(4) COLLATE UTF8_LCASE)) AS c""".stripMargin)
+ .schema.head.dataType === CharType(4, "UTF8_LCASE"))
+
+ // Set ops and multi-row VALUES share the same LCT.
+ assert(sql(
+ """SELECT c FROM (
+ | SELECT cast('a' AS CHAR(2)) AS c UNION SELECT cast('a' AS
CHAR(4)) AS c) t"""
+ .stripMargin)
+ .schema.head.dataType === CharType(4))
+ checkAnswer(
+ sql("""SELECT c FROM (
+ | SELECT cast('a' AS CHAR(2)) AS c UNION SELECT cast('a' AS
CHAR(4)) AS c) t"""
+ .stripMargin),
+ Row("a "))
+ checkAnswer(
+ sql("""SELECT c FROM (
+ | SELECT cast('ab' AS CHAR(2)) AS c INTERSECT
+ | SELECT cast('ab' AS CHAR(4)) AS c) t""".stripMargin),
+ Row("ab "))
+ checkAnswer(
+ sql("""SELECT c FROM (
+ | SELECT cast('ab' AS CHAR(2)) AS c EXCEPT
+ | SELECT cast('ab' AS CHAR(4)) AS c) t""".stripMargin),
+ Seq.empty)
+ assert(sql(
+ """SELECT c FROM (VALUES (cast('a' AS CHAR(2))), (cast('bb' AS
CHAR(4)))) t(c)""")
+ .schema.head.dataType === CharType(4))
+ }
+ }
+
+ test("SPARK-58794: parameterized CHAR/VARCHAR lengths under
standardSemantics") {
+ // Length positions accept parameter markers (`integerValue` ->
`parameterMarker`). Under
+ // standardSemantics the bound type stays first-class: CAST keeps
CHAR/VARCHAR, pads and
+ // enforces length, and DDL schemas retain the substituted n.
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val charDf = spark.sql("SELECT cast('ab' AS CHAR(:n)) AS c", Map("n" ->
5))
+ assert(charDf.schema.head.dataType === CharType(5))
+ checkAnswer(
+ spark.sql("SELECT concat('<', cast('ab' AS CHAR(:n)), '>')", Map("n"
-> 5)),
+ Row("<ab >"))
+
+ val varcharDf = spark.sql("SELECT cast('hello' AS VARCHAR(?)) AS c",
Array(5))
+ assert(varcharDf.schema.head.dataType === VarcharType(5))
+ intercept[SparkRuntimeException] {
+ spark.sql("SELECT cast('abcdef' AS VARCHAR(?))", Array(2)).collect()
+ }
+
+ withTable("param_varchar", "param_char") {
+ spark.sql(
+ "CREATE TABLE param_varchar (c VARCHAR(:n)) USING parquet", Map("n"
-> 7))
+ assert(spark.table("param_varchar").schema.head.dataType ===
VarcharType(7))
+ spark.sql("CREATE TABLE param_char (c CHAR(?)) USING parquet",
Array(4))
+ assert(spark.table("param_char").schema.head.dataType === CharType(4))
+ }
+
+ // Non-integral / negative lengths fail when substituted into the length
position.
+ intercept[ParseException] {
+ spark.sql("SELECT cast('a' AS CHAR(:n))", Map("n" -> -1)).collect()
+ }
+ intercept[ParseException] {
+ spark.sql("SELECT cast('a' AS CHAR(:n))", Map("n" -> 1.5)).collect()
+ }
+ }
+ }
+
test("SPARK-58802: single-pass resolver agrees with fixed-point under
standardSemantics") {
Review Comment:
Added to the SPARK-58802 dual-run matrix: collated mixed-length COALESCE
(equal strength and Implicit vs Default), collated compare / IN, and mixed-CHAR
`UNION ALL`.
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar-standard-semantics.sql:
##########
@@ -66,22 +66,74 @@ SELECT typeof(reverse(array(1, 2)));
SELECT typeof(str_to_map(cast('a:1,b:2' AS CHAR(7))));
SELECT typeof(c0) FROM (SELECT json_tuple(cast('{"a":"1"}' AS CHAR(9)), 'a')
AS c0);
--- R2 with collation. A declared collation survives the CAST and an LCT over
equally constrained
--- operands. The mixed-length case (CHAR(2) with CHAR(4), same collation) is
deliberately not
--- covered here: CollationTypeCoercion reads the differing lengths as a
collation mismatch and
--- yields an indeterminate collation. That predates this change (it reproduces
under
--- spark.sql.preserveCharVarcharTypeInfo) and is tracked separately, so
goldening it would
--- normalize the bug.
+-- Collation survives CAST and LCT. Mixed lengths with the same collation
widen to max(n, m);
+-- they must not collapse to an indeterminate collation.
SELECT typeof(cast('a' AS CHAR(2) COLLATE UTF8_LCASE));
SELECT typeof(coalesce(
cast('a' AS CHAR(2) COLLATE UTF8_LCASE), cast('bb' AS CHAR(2) COLLATE
UTF8_LCASE)));
+SELECT typeof(coalesce(
+ cast('a' AS CHAR(2) COLLATE UTF8_LCASE), cast('bb' AS CHAR(4) COLLATE
UTF8_LCASE)));
+SELECT concat('<', coalesce(
+ cast('a' AS CHAR(2) COLLATE UTF8_LCASE), cast('bb' AS CHAR(4) COLLATE
UTF8_LCASE)), '>');
--- UNION LCT
+-- Set operations and multi-row VALUES share the same LCT as COALESCE.
SELECT typeof(c) FROM (
SELECT cast('a' AS VARCHAR(3)) AS c
UNION ALL
Review Comment:
Updated the golden matrix:
- mixed-CHAR `UNION ALL` (typeof + padded values)
- collated CHAR/VARCHAR COALESCE, compare, and IN
- Implicit vs Default mixed-length COALESCE (the remaining narrowing case)
- non-empty EXCEPT (`CHAR(2) 'ab'` vs `CHAR(4) 'xy'`) with `concat` so the
padding is visible
- three-part IN in the analyzer golden
--
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]
