Re: [PR] [SPARK-58798][SQL] Fix collated CHAR LCT and cover compare/IN/set-ops [spark]

["srielau (via GitHub)" <gi...@apache.org>]

srielau commented on code in PR #58080:
URL: https://github.com/apache/spark/pull/58080#discussion_r3820573515


##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java:
##########
@@ -54,6 +54,24 @@ public static UTF8String varcharTypeWriteSideCheck(UTF8String inputStr, int limi
     }
   }
 
+  /**
+   * Applies the SQL explicit-cast rules for a character string source and CHAR target.
+   *
+   * Unlike store assignment, an explicit character-to-character cast truncates non-space
+   * characters instead of raising a right-truncation exception.
+   */
+  public static UTF8String charTypeCast(UTF8String inputStr, int limit) {
+    UTF8String truncated = varcharTypeCast(inputStr, limit);
+    return truncated.numChars() < limit ? truncated.rpad(limit, SPACE) : truncated;

Review Comment:
   Done in `de631ae`. `charTypeCast` now computes `numChars` once and branches to return, pad, or `substring(0, limit)`. Over-limit values are no longer scanned again after `varcharTypeCast`.



##########
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 widening, 'ab  ' is not 'xy  '.
+SELECT typeof(c) FROM (
+  SELECT cast('ab' AS CHAR(2)) AS c
+  EXCEPT
+  SELECT cast('xy' AS CHAR(4)) AS c
+) t;
+SELECT concat('<', c, '>') FROM (
+  SELECT cast('ab' AS CHAR(2)) AS c
+  EXCEPT
+  SELECT cast('xy' AS CHAR(4)) AS c
+) t;
+SELECT typeof(c) FROM (VALUES
+  (cast('a' AS CHAR(2))),
+  (cast('bb' AS CHAR(4)))
+) t(c);
+SELECT concat('<', c, '>') FROM (VALUES
+  (cast('a' AS CHAR(2))),
+  (cast('bb' AS CHAR(4)))
+) t(c);
+
+-- Comparison and IN: both sides (including the IN left-hand side) are cast to the LCT of all
+-- participants. Casting to CHAR pads, so CHAR vs CHAR of different lengths compares equal after
+-- widen; casting to VARCHAR/STRING keeps the CHAR pad, so CHAR 'a' (stored as 'a ') is not equal

Review Comment:
   Applied in `de631ae`.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org