srielau commented on code in PR #58080:
URL: https://github.com/apache/spark/pull/58080#discussion_r3817208824
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala:
##########
@@ -184,13 +184,39 @@ object Project {
if (other == target) {
col
} else if (Cast.canANSIStoreAssign(other, target)) {
- Cast(col, target, Option(conf.sessionLocalTimeZone), ansiEnabled =
true)
+ storeAssignCast(col, other, target, conf)
} else {
throw
QueryCompilationErrors.invalidColumnOrFieldDataTypeError(columnPath, other,
target)
}
}
}
+ // Store assignment must not use character-to-character CAST truncation (ISO
6.13).
+ // Cast to unconstrained STRING first, then apply the write-side length
check.
+ // Avoid replaceCharVarcharWithString: first-class types keep CHAR/VARCHAR.
+ private def storeAssignCast(
Review Comment:
Thanks for extracting this and using `shouldApplyWriteSideLengthCheck`. The
three-line comment is the helper's contract -- could it be Scaladoc on
`storeAssignCast`, with the STRING-then-write-check plan shape?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCoercion.scala:
##########
@@ -115,7 +122,8 @@ object CollationTypeCoercion extends SQLConfHelper {
expr match {
case lit: Literal => lit.copy(dataType = newDataType)
Review Comment:
Cast nesting looks right (goldens show `cast(cast(abcdef as varchar(2)) as
varchar(4))` and result `ab`).
This Literal arm still retargets without applying the new constraint.
`lit.copy` was reasonable when only collation changed; LCT can now change
CHAR/VARCHAR length. `Literal.create("a", CharType(2))` stores `'a '`; copying
to `CharType(4)` leaves `'a '` rather than `'a '`. A narrower VARCHAR
selected by collation precedence would skip the length check the same way.
SQL CAST is a `Cast` node so it does not hit this. Please wrap typed
CHAR/VARCHAR literals in an untagged outer Cast when the constraint changes,
and add a focused `Literal.create` test (SQL text cannot construct that node).
--
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]
