srielau commented on code in PR #58080:
URL: https://github.com/apache/spark/pull/58080#discussion_r3818556952
##########
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:
Done in `d096008`. The converter is now bound once when the lambda is
constructed (`val toUTF8String = castToString(from)`) instead of being
re-selected and re-built per interpreted row.
Both call sites build it once per expression -- `Cast`'s `lazy val cast` and
`ToPrettyString.castFunc` -- so this takes converter construction off the row
path entirely.
##########
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:
Done in `d096008`. The struct arm now iterates matching field indices, so
there is no `zip` tuple array; `exists` short-circuiting and field order are
unchanged.
##########
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:
Applied in `d096008`.
--
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]