cloud-fan commented on code in PR #58080:
URL: https://github.com/apache/spark/pull/58080#discussion_r3818137996
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCoercion.scala:
##########
@@ -127,6 +140,24 @@ object CollationTypeCoercion extends SQLConfHelper {
}
}
+ /**
+ * True when CHAR/VARCHAR length (or nested length) differs between `from`
and `to`.
+ * Collation-only differences are not a constraint change.
+ */
+ private def stringConstraintChanged(from: DataType, to: DataType): Boolean =
{
+ (from, to) match {
+ case (f: StringType, t: StringType) => f.constraint != t.constraint
+ case (ArrayType(fe, _), ArrayType(te, _)) => stringConstraintChanged(fe,
te)
+ case (MapType(fk, fv, _), MapType(tk, tv, _)) =>
+ stringConstraintChanged(fk, tk) || stringConstraintChanged(fv, tv)
+ case (fs: StructType, ts: StructType) if fs.length == ts.length =>
+ fs.fields.zip(ts.fields).exists { case (a, b) =>
Review Comment:
**Non-blocking:**
Avoid materializing a tuple array for every nested struct comparison.
Iterating matching field indices preserves `exists` short-circuiting and field
order without the `zip` allocation.
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar-standard-semantics.sql:
##########
@@ -66,22 +79,108 @@ 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 hex(coalesce(
+ cast('a' AS CHAR(2) COLLATE UTF8_LCASE), cast('bb' AS CHAR(4) COLLATE
UTF8_LCASE)));
+SELECT typeof(coalesce(
+ cast('a' AS CHAR(2) COLLATE UTF8_LCASE), cast('bb' AS VARCHAR(4) COLLATE
UTF8_LCASE)));
+-- Mixed strength, same collation: Implicit string CAST CHAR(2) vs Default
+-- non-string CAST CHAR(4). Length still widens to max(n, m); the COLLATE
+-- operator itself is STRING, so it is not used here.
+SELECT typeof(coalesce(
+ cast('a' AS CHAR(2) COLLATE UTF8_LCASE),
+ cast(1 AS CHAR(4) COLLATE UTF8_LCASE)));
+SELECT hex(coalesce(
+ cast('a' AS CHAR(2) COLLATE UTF8_LCASE),
+ cast(1 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
SELECT cast('abcd' AS VARCHAR(8)) AS c
) t LIMIT 1;
+SELECT typeof(c) FROM (
+ SELECT cast('a' AS CHAR(2)) AS c
+ UNION ALL
+ SELECT cast('bb' AS CHAR(4)) AS c
+) t LIMIT 1;
+SELECT concat('<', c, '>') FROM (
+ SELECT cast('a' AS CHAR(2)) AS c
+ UNION ALL
+ SELECT cast('bb' AS CHAR(4)) AS c
+) t;
+SELECT typeof(c) FROM (
+ SELECT cast('a' AS CHAR(2)) AS c
+ UNION
+ SELECT cast('a' AS CHAR(4)) AS c
+) t;
+SELECT concat('<', c, '>') FROM (
+ SELECT cast('a' AS CHAR(2)) AS c
+ UNION
+ SELECT cast('a' AS CHAR(4)) AS c
+) t;
+SELECT typeof(c) FROM (
+ SELECT cast('ab' AS CHAR(2)) AS c
+ INTERSECT
+ SELECT cast('ab' AS CHAR(4)) AS c
+) t;
+SELECT concat('<', c, '>') FROM (
+ SELECT cast('ab' AS CHAR(2)) AS c
+ INTERSECT
+ SELECT cast('ab' AS CHAR(4)) AS c
+) t;
+-- Non-empty EXCEPT: after widen, 'ab ' is not 'xy '.
Review Comment:
**Nit:**
```suggestion
-- Non-empty EXCEPT: after widening, 'ab ' is not 'xy '.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ToStringBase.scala:
##########
@@ -58,12 +65,18 @@ trait ToStringBase { self: UnaryExpression with
TimeZoneAwareExpression =>
// Returns a function to convert a value to pretty string. The function
assumes input is not null.
protected final def castToString(
from: DataType, to: StringConstraint = NoConstraint): Any => UTF8String =
- to match {
- case FixedLength(length) =>
+ (to, from) match {
+ case (FixedLength(length), _: StringType)
+ if SQLConf.get.charVarcharStandardSemantics &&
truncateCharVarcharOnCast =>
+ s => CharVarcharCodegenUtils.charTypeCast(castToString(from)(s),
length)
Review Comment:
**Non-blocking:**
Build the source-to-UTF8 converter once when constructing this lambda.
`castToString(from)` currently repeats converter selection and construction for
every interpreted row; cache it in a local value and call that converter here.
--
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]
