srielau commented on code in PR #58080:
URL: https://github.com/apache/spark/pull/58080#discussion_r3817090734
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCoercion.scala:
##########
@@ -115,6 +123,8 @@ object CollationTypeCoercion extends SQLConfHelper {
expr match {
case lit: Literal => lit.copy(dataType = newDataType)
+ case cast: Cast if needsCharPaddingCast(cast.dataType, newDataType)
=>
+ Cast(cast, newDataType, timeZoneId =
Some(conf.sessionLocalTimeZone))
case cast: Cast => cast.copy(dataType = newDataType)
Review Comment:
Fixed in dddd721.
`changeType` always nests `Cast(existingCast, lct)` and no longer retargets.
Analyzer goldens for `COALESCE(CAST('abcdef' AS VARCHAR(2)), CAST('x' AS
VARCHAR(4)))` now show `cast(cast(abcdef as varchar(2)) as varchar(4))` and the
result is `ab`. Numeric `CAST(12345 AS VARCHAR(4))` inside COALESCE still
raises `EXCEED_LIMIT_LENGTH`; `try_cast` still yields NULL then the sibling.
Same coverage for CASE / IN and a collated VARCHAR COALESCE. Those cases are
also in the SPARK-58802 dual-run matrix.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala:
##########
@@ -184,7 +184,24 @@ object Project {
if (other == target) {
col
} else if (Cast.canANSIStoreAssign(other, target)) {
- Cast(col, target, Option(conf.sessionLocalTimeZone), ansiEnabled =
true)
+ // 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.
+ val (castTarget, lengthCheckType) = target match {
+ case c: CharType => (c.toStringType, Some(c: DataType))
+ case v: VarcharType => (v.toStringType, Some(v: DataType))
+ case otherType => (otherType, None)
+ }
+ val casted = if (other == castTarget) {
+ col
+ } else {
+ Cast(col, castTarget, Option(conf.sessionLocalTimeZone),
ansiEnabled = true)
+ }
+ if (lengthCheckType.isDefined && !conf.charVarcharAsString) {
Review Comment:
Fixed in dddd721.
Write-side length checks now use
`CharVarcharUtils.shouldApplyWriteSideLengthCheck`: skip only on the legacy
`charVarcharAsString` path without first-class types. `standardSemantics` (and
preserve) still apply the check when the legacy flag is also on.
`TableOutputResolver` and default-column evaluation use the same helper. Added
a combined-flag `Dataset.to` / `INSERT` regression test.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ToStringBase.scala:
##########
@@ -58,12 +58,16 @@ 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 =>
+ s => CharVarcharCodegenUtils.charTypeCast(castToString(from)(s),
length)
+ case (MaxLength(length), _: StringType) if
SQLConf.get.charVarcharStandardSemantics =>
Review Comment:
Fixed in dddd721.
Truncation in `ToStringBase` is gated on `Cast.USER_SPECIFIED_CAST` (already
set by the parser, `Column.cast`, and Connect). Implicit / store-assignment
Casts keep the write-side length check. `ToPrettyString` stays on the
write-side path. Project still goes through STRING then `stringLengthCheck` so
`Dataset.to` does not depend on a tagged Cast.
--
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]
