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

[srielau]

What changes were proposed in this pull request?

Covers the string-family LCT lattice (SPARK-58798). Parent: SPARK-58794. Also includes explicit character CAST truncation (SPARK-58797).

Rebased onto master after #58033 merged. Unique commits vs master: srielau/spark@master...SPARK-58794-lct-compare

Follow-up that drills into least-common-type, comparison, and IN under spark.sql.charVarchar.standardSemantics.enabled.

CollationTypeCoercion is not gated on standardSemantics alone (charVarcharFirstClassTypes is also true under preserveCharVarcharTypeInfo). The equal-collation length LCT therefore applies in preserve-only mode as well; tests cover both flags.

Bug fix.CollationTypeCoercion.getWinningStringType required sameType at equal collation strength. Same-collation CHAR(2) and CHAR(4) therefore looked like a collation mismatch and became IndeterminateStringType (string collate null). At equal strength, take StringHelper.tightestCommonString instead so every child is cast to max(n, m) (padding, never truncating).

Explicit CAST vs store assignment (ISO 6.13 / 9.2). Under the flag:

• Character-to-CHAR/VARCHARCAST / TRY_CASTtruncates to n characters (then CHAR pads). Example: CAST('abcdef' AS VARCHAR(2)) is 'ab', not EXCEED_LIMIT_LENGTH.
• Numeric (and other non-string) CAST to CHAR/VARCHAR still errors when the formatted literal does not fit (CAST(12345 AS VARCHAR(4)) -> EXCEED_LIMIT_LENGTH / 54006). That matches ISO 22001, not silent digit clipping.
• Store assignment (INSERT, Dataset.to, encoder write-side) is unchanged: non-space overflow still errors. Project store-assignment casts to unconstrained STRING then stringLengthCheck, so they do not inherit CAST truncation.

Coverage / pinned behavior.

• Set ops: UNION / UNION ALL / INTERSECT / EXCEPT, plus multi-row VALUES, all share the string-family LCT.
• Comparison and IN: every participant is cast to that LCT, including the IN left-hand side (InTypeCoercion uses findWiderCommonType over value +: list). Casting to CHAR pads, so unequal CHAR lengths compare equal after widen; casting to VARCHAR/STRING keeps the CHAR pad, so CHAR 'a' (stored as 'a ') is not equal to VARCHAR/STRING 'a' unless the other side carries the same trailing blank. Ignoring trailing blanks is a collation concern (RTRIM), not a type-level PAD SPACE policy (design D17).
• Parameterized lengths (CHAR(:n) / VARCHAR(?)): markers in length position already bind through the parser; under the flag, character CAST truncates to the bound n, numeric CAST still errors if it does not fit, and DDL keeps the bound n. Negative / non-integral lengths fail at parse after substitution.

Why are the changes needed?

#58033 left the collated mixed-length LCT gap as a known issue, and had only thin coverage of UNION ALL / INTERSECT with no compare / IN-LHS / VALUES / EXCEPT matrix. Without pinning the LCT-cast rule, CHAR vs CHAR vs VARCHAR equality is easy to misread as PAD SPACE.

Explicit character CAST was still using the write-side length check, so CAST('abcdef' AS VARCHAR(2)) failed the same way as INSERT. ISO 6.13 truncates that CAST; 9.2 keeps the INSERT error. Numeric CAST must still fail when the literal does not fit (ISO 22001).

Does this PR introduce any user-facing change?

Yes, when the flag is on:

• Collated CHAR/VARCHAR values of different lengths but the same collation now widen to CHAR/VARCHAR(max(n,m)) instead of an indeterminate collation. Compare / IN / set-op results for mixed CHAR lengths follow the LCT-cast rule above (this matches what the engine already did for non-collated paths; the new tests lock it in).
• Explicit character-to-CHAR/VARCHAR CAST truncates instead of raising EXCEED_LIMIT_LENGTH. INSERT / encoder overflow and numeric CAST overflow still error.

How was this patch tested?

• New BasicCharVarcharTestSuite cases covering compare, IN (including analyzed-plan assertion that the LHS widens), RTRIM, collated LCT, UNION / INTERSECT / EXCEPT, multi-row VALUES, parameterized CHAR/VARCHAR lengths, character CAST truncation, and numeric CAST overflow under the flag.
• Expanded golden file charvarchar-standard-semantics.sql.
• Locally: BasicCharVarcharTestSuite and the golden file regeneration/run.

[srielau]

SQL Language review

Domain review of the collated CHAR LCT fix and compare/IN/set-op coverage.

Surface areas: fixed-point Analyzer (CollationTypeCoercion), SQL golden files, BasicCharVarcharTestSuite. Analyzer++ is not directly edited; it shares expression coercion but set-op resolution is a separate path.

Findings

1. Critical -- collation precedence can still narrow length.tightestCommonString runs only on equal strength. The unequal-priority branch still returns the entire winning StringTypeWithContext, so an explicit CHAR(2) COLLATE UTF8_LCASE against an implicit same-collation CHAR(4) selects CHAR(2). That contradicts the stated invariant (same-collation operands widen to max(n, m), never truncate). Collation strength and string-family LCT should be resolved independently.

2. Medium -- parameterized-length test is out of scope. It does not exercise getWinningStringType. Please drop it from this PR (or move missing CAST/error cases to a parameter suite with checkError).

3. Medium -- unit tests duplicate goldens; plan substring is weak. Most of the SPARK-58794 unit test repeats charvarchar-standard-semantics.sql. analyzed.toString.contains("as varchar(4)") can match a list-element cast and does not prove the IN LHS widened. Put that three-part IN query in the analyzer golden instead.

4. Medium -- the actual regression is not in the SPARK-58802 dual-run matrix. Please add collated mixed-length COALESCE / compare / IN, and mixed-CHAR UNION ALL, under dual-run.

5. Medium -- golden matrix does not match the PR description.UNION ALL is still VARCHAR(3)/VARCHAR(8); new set-ops are uncollated; no collated mixed CHAR/VARCHAR; EXCEPT has an empty result so padding is not observed.

6. Medium -- not standardSemantics-only.charVarcharFirstClassTypes is also true under preserveCharVarcharTypeInfo, and CollationTypeCoercion is always installed. Document and test preserve-only behavior, or gate if preserve must not change.

7. Low -- load-bearing comment belongs on the method scaladoc, with a concrete SQL / type example.

Suggested order

1. Split collation-strength selection from CHAR/VARCHAR length LCT; add explicit-vs-implicit mixed-length goldens.
2. Dual-run the actual collated mixed-length cases.
3. Align goldens with UNION ALL / collated CHAR-VARCHAR / non-empty EXCEPT.
4. Remove duplicated unit coverage and the parameterized-length test.
5. Call out preserve-mode impact in the PR description.

[srielau]

SQL Language re-review (64008e1)

Previous findings 1-3 and 5-7 are addressed. The same-collation strength/length split is fixed: tightestCommonString whenever collation ids match, then attach the stronger strength. Implicit vs Default mixed-length COALESCE is covered in goldens, preserve-only tests, and dual-run. Parameterized-length coverage (#4) kept with checkError -- not blocking.

One new correctness issue is reachable because that LCT now succeeds.

Remaining

1. Critical -- collated CHAR-to-VARCHAR LCT rewrites the inner Cast and drops padding.changeType does cast.copy(dataType = newDataType). Uncollated TypeCoercion keeps a nested cast (cast(cast(a as char(2)) as varchar(2)) = ... -> false). After this fix, collated CHAR vs VARCHAR goldens retarget to cast(a as varchar(2) collate UTF8_LCASE) on both sides and result true. UTF8_LCASE is not RTRIM; the PR text still says VARCHAR/STRING keep the CHAR pad. Dual-run only checks collated CHAR vs CHAR, so it will not catch this.

2. Low -- different collations at unequal strength still return the full winner (length included). Pre-existing SQL collation precedence; documented in the new scaladoc. Not a merge blocker.

Please wrap instead of retargeting when the source is CHAR and the LCT is not, flip those two goldens to false, and add that compare/IN pair to SPARK-58802 dual-run.

[srielau]

SQL Language re-review (0c8e47f)

Previous CHAR-pad finding: PARTIAL. The reported collated CHAR vs VARCHAR case is fixed: analyzer nests cast(cast(a as char(2) collate UTF8_LCASE) as varchar(2) ...), execution is false. changeType still retargets every other Cast, which now interacts with truncating explicit CAST.

Findings

1. Critical -- retargeting still erases explicit CAST semantics. Only CHAR-to-non-CHAR is nested. CAST('abcdef' AS VARCHAR(2)) inside COALESCE/CASE/IN with a wider VARCHAR sibling can become CAST('abcdef' AS VARCHAR(4)) and yield abcd instead of ab. Numeric / TRY_CAST overflow can disappear the same way. ISO 6.13 then depends on surrounding LCT.

2. Critical -- store check skipped when standardSemantics and charVarcharAsString are both on.stringLengthCheck is gated on !conf.charVarcharAsString, but standardSemantics keeps first-class CHAR/VARCHAR when both flags are true. Dataset.to can then skip the write-side check. TableOutputResolver uses the same gate (pre-existing); this PR copies it into Project.

3. High -- ToStringBase cannot tell explicit CAST from store/implicit Cast. Truncation is implemented on all string-to-CHAR/VARCHAR Casts; Project is a reach-around so INSERT/Dataset.to do not inherit it. Other write paths can drift.

4. Medium -- CAST truncation + store-assignment rewrite is a second, user-visible change. Consider splitting from the collated-LCT fix.

5. Medium -- tests miss the interacting paths: truncating CAST/TRY_CAST and numeric overflow inside COALESCE/CASE/IN; combined-flag Dataset.to / INSERT; collated truncating CAST; dual-run of truncation+LCT.

6. Low -- extract the Project store-assignment helper (pattern match, not isDefined/get).

Please preserve inner constrained-string Casts in changeType (wrap, do not retarget), fix the combined-flag gate, and add those goldens. The CHAR-pad compare/IN goldens look correct now.

[srielau]

SQL Language re-review (dddd721)

Previous review comments are addressed for the SQL CAST / store-assignment paths.

#	Original	Status
1	changeType retargets Cast (VARCHAR truncate / numeric overflow)	Fixed -- always Cast(existing, lct); COALESCE/CASE/IN goldens nest and return ab; numeric COALESCE still EXCEED_LIMIT_LENGTH
2	Write-side check skipped when both flags are on	Fixed -- shouldApplyWriteSideLengthCheck; Project, TableOutputResolver, default columns
3	ToStringBase cannot tell explicit CAST from store Cast	Fixed -- truncateCharVarcharOnCast / USER_SPECIFIED_CAST; outer LCT Cast is untagged
4	Split CAST truncation from LCT	Open / non-blocking -- maintainer scope
5	Missing CAST-inside-LCT tests	Fixed
6	Extract Project helper	Fixed (storeAssignCast); scaladoc nit remains

Remaining

1. Medium -- Literal arm still retargets.lit.copy(dataType = newDataType) does not rematerialize CHAR pad / VARCHAR length. SQL CAST stays a Cast so the new goldens are fine; Literal.create(..., CharType(2)) widened to CHAR(4) keeps 'a ' instead of 'a '. Please wrap when the string constraint changes (untagged outer Cast), and add a rule-level Literal.create test.

2. Low -- storeAssignCast contract is still an inline comment. Lift it to Scaladoc with the STRING-then-write-check shape.

CHAR-pad compare/IN and CAST-inside-LCT goldens look correct. Not blocking on (2) or the CAST/LCT scope split.

[cloud-fan]

0 blocking, 2 non-blocking, 1 nit.
The semantic fixes and regression coverage are coherent; three small performance/prose cleanups remain.

Suggestions (2)

• Non-blocking: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ToStringBase.scala:71: Cache the source-to-UTF8 converter outside the per-row lambda. -- see inline
• Non-blocking: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCoercion.scala:154: Compare matching struct fields by index to avoid an eager zipped tuple array. -- see inline

Nits: 1 minor item (see inline comments).

Verification

I traced explicit-cast tagging from parser/Column/Connect into Cast and both interpreted/code-generated ToStringBase paths; traced analyzer LCT wrapping and typed-Literal rematerialization; and checked store-assignment consumers against the shared write-side predicate. The generated SQL dossier and focused suite assert the resulting schemas, values, errors, padding, and analyzer parity.

[cloud-fan]

3 addressed, 0 remaining, 2 new to this AI review. (0 newly introduced, 2 late catches, 0 previously raised.)
0 blocking, 1 non-blocking, 1 nit.
The semantic fix is coherent; one small hot-path optimization and one comment grammar correction remain.

Suggestions (1)

• Non-blocking: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java:65: Compute the CHAR source length once so truncating casts avoid a second UTF8 traversal. -- see inline

Nits: 1 minor item (see inline comments).

Verification

I traced explicit-cast tagging from SQL parsing, classic Column.cast, and Connect through Cast into both interpreted and generated ToStringBase paths. I also verified that CollationTypeCoercion preserves inner cast semantics, and that Project, TableOutputResolver, and default-column validation share the write-side length-check predicate. The generated SQL dossier confirms the corresponding schemas, values, errors, padding, and analyzer plans.

[cloud-fan]

2 addressed, 0 remaining, 0 new to this AI review.
0 blocking, 0 non-blocking, 0 nits.
The current head is coherent and the two issues from the prior AI review are addressed; I found no remaining defects.

Verification

I traced explicit-cast provenance from SQL parsing, classic Column.cast, and Connect through Cast into both interpreted and generated ToStringBase paths. I also verified that CollationTypeCoercion preserves inner cast semantics, and that Project, TableOutputResolver, and default-column validation share the write-side length-check predicate. The generated SQL dossier and focused suite cover the resulting schemas, values, errors, padding, and analyzer plans.

[Yicong-Huang]

Merge Summary:

• merged into master ac5bba0
• merged into branch-4.x 7f73f99

Posted by merge_spark_pr.py