srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3801131958
##########
sql/api/src/main/scala/org/apache/spark/sql/types/StringType.scala:
##########
@@ -159,25 +159,76 @@ case object StringHelper extends
PartialOrdering[StringConstraint] {
def isPlainString(s: StringType): Boolean = s.constraint == NoConstraint
+ /**
+ * Strip CHAR/VARCHAR length constraints, preserving collation.
+ *
+ * Used by transforming string expressions (upper, substr, concat, ...) so
their result type is
+ * plain STRING even when inputs are CharType/VarcharType (SQL standard
CHAR/VARCHAR R1), when
+ * standard semantics are on.
+ */
+ def plainStringType(dt: DataType): DataType = dt match {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
+ }
+
+ def plainStringType(s: StringType): StringType = s match {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
+ }
+
+ /**
+ * Result type for transforming string expressions. Under
+ * spark.sql.charVarchar.standardSemantics.enabled, always plain STRING
(R1). Under
+ * preserveCharVarcharTypeInfo alone, keep child type (legacy leaky path).
+ */
+ def transformingStringResultType(dt: DataType): DataType = {
Review Comment:
Agreed on the diagnosis, and the sweep found more than `StringToMap`:
`json_tuple` leaked the same way (both take their input through
`ExpectsInputTypes`, so no implicit cast strips the length). Both now reduce
the extracted values to plain STRING.
On centralizing: `charVarcharToPlainString` in `ImplicitTypeCasts` already
is the central mechanism for everything that goes through implicit casting,
which is the large majority. What it structurally cannot reach is
`ExpectsInputTypes` (no cast is inserted) and expressions that derive a result
type from a child without a cast. A trait would not close that either, since
the leak is in the `dataType` override rather than in the input contract, so I
would rather not add a second mechanism on top of the two we have.
What I did add is the enforcement you asked for: `SPARK-58794: no unlisted
function returns a CHAR/VARCHAR type under standardSemantics` sweeps the whole
`FunctionRegistry` with several argument shapes and fails on any function that
returns a constrained type unless it is in an explicit pass-through allowlist
(the R2/R3 set: aggregates returning an input unchanged, the null-handling
family, element access, and the array/map/struct constructors). A newly added
expression that leaks now fails the test instead of shipping; a genuine
pass-through requires a deliberate allowlist entry.
--
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]
