cloud-fan commented on code in PR #58549:
URL: https://github.com/apache/spark/pull/58549#discussion_r4015150601
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/UserDefinedPythonFunction.scala:
##########
@@ -108,7 +108,22 @@ case class UserDefinedPythonFunction(
}
PythonAggregate(name, func, dataType, e, udfDeterministic, bufferStruct)
} else {
- PythonUDF(name, func, dataType, e, pythonEvalType, udfDeterministic)
+ val applyCharVarcharChecks =
+ CharVarcharUtils.shouldApplyWriteSideLengthCheck(SQLConf.get)
+ val resolvedDataType = if (applyCharVarcharChecks) {
Review Comment:
**Blocking (P1):** `shouldApplyWriteSideLengthCheck` is true under the
default flags even though `charVarcharFirstClassTypes` is false. This branch
therefore leaves CHAR/VARCHAR on the `PythonUDF` result for newly supported
Arrow scalar UDFs, exposing first-class constrained types where the default
contract requires annotated STRING. Please choose the exposed result type from
the first-class-type policy independently of the captured boolean that controls
value checks.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala:
##########
@@ -33,6 +33,28 @@ object CharVarcharUtils extends Logging with
SparkCharVarcharUtils {
// visible for testing
private[sql] val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY =
"__CHAR_VARCHAR_TYPE_STRING"
+ /**
+ * Replaces CHAR/VARCHAR with their unconstrained string representation
regardless of session
+ * configuration. Use this only at physical boundaries, such as Arrow, that
encode all character
+ * string types as UTF8.
+ */
+ private[sql] def replaceCharVarcharWithStringForPhysicalType(dt: DataType):
DataType = dt match {
+ case ArrayType(elementType, containsNull) =>
+ ArrayType(replaceCharVarcharWithStringForPhysicalType(elementType),
containsNull)
+ case MapType(keyType, valueType, valueContainsNull) =>
+ MapType(
+ replaceCharVarcharWithStringForPhysicalType(keyType),
+ replaceCharVarcharWithStringForPhysicalType(valueType),
+ valueContainsNull)
+ case StructType(fields) =>
+ StructType(fields.map { field =>
+ field.copy(dataType =
replaceCharVarcharWithStringForPhysicalType(field.dataType))
+ })
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
Review Comment:
**Blocking (P1):** This physical-boundary recursion stops at
`UserDefinedType`, even though Python's Arrow mapping descends into the UDT's
`sqlType`. A UDT whose storage schema contains CHAR/VARCHAR is therefore
decoded without physical normalization or activating the checked projection, so
standard-mode creation can retain unpadded or over-length values. The same
leaf-only predicate is used by the state and analyze-derived UDTF guards,
allowing schemas those paths promise to reject.
**Recommended change:** Add a UDT-aware physical CHAR/VARCHAR traversal and
use it consistently for Arrow physical schema conversion and assignment checks
and for TransformWithState and analyze-derived Python UDTF rejection; add
focused UDT-storage regressions for the supported and unsupported boundaries.
**Why this works:** At physical boundaries, descend through
UserDefinedType.sqlType while retaining the UDT in the public logical schema.
Convert the storage schema to physical STRING, detect constrained leaves
through that storage schema, and build assignment expressions against the
storage representation for supported Arrow creation. Route the state and UDTF
admission checks through the same complete predicate so they reject before
execution.
**Scope:** Make JVM CHAR/VARCHAR boundary handling recurse through UDT
storage without changing the public logical UDT contract.
**Compatibility:** Built-in struct, array, and map recursion, legacy
unchecked conversion, ordinary UDT transport, and the current unsupported
status of constrained-string state and UDTF schemas remain unchanged.
**Risks:** A broadly changed generic UDT predicate could affect non-Arrow
catalog or analyzer behavior that intentionally treats UDT as one logical type.
Assignment expressions must operate on the UDT storage representation while
preserving nullability and the public logical UDT schema.
**Constraints:** Keep UDT-aware behavior scoped to physical Python and Arrow
boundaries unless a broader semantic audit justifies changing the generic
predicate. Preserve the configured legacy no-check path and the logical UDT
wrapper exposed to users. Keep state and UDTF APIs that do not support
constrained strings on their existing deterministic rejection path.
**Success:** Arrow explicit-schema creation applies CHAR padding and rejects
over-length VARCHAR values nested in UDT storage under write-check semantics in
both local and RDD relation branches. TransformWithState schemas and
analyze-derived Python UDTF schemas reject CHAR/VARCHAR nested in UDT storage
before execution. UDTs without constrained-string storage retain their existing
transport, schema, and execution behavior.
--
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]
