cloud-fan commented on code in PR #58936:
URL: https://github.com/apache/spark/pull/58936#discussion_r4186070318
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -62,6 +80,12 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
override def outputPartitioning: Partitioning = child.outputPartitioning
+ // Snapshot at plan construction. CHAR_VARCHAR_STANDARD_SEMANTICS is
PERSISTED, so a
+ // plan built under one setting must keep that path even if a later task
SQLConf.get
+ // (ReadOnlySQLConf with the executing session) differs. SparkPlan.session is
+ // @transient, so executor `conf` cannot recover the planning session.
+ private val standardCharVarcharSemantics: Boolean =
conf.charVarcharStandardSemantics
Review Comment:
**Non-blocking (P2):** This snapshot is taken too late for the config's
persisted-view contract. A view body is resolved under its saved SQLConf, but
that scope has ended before the physical script operator is constructed, so a
disabled caller makes this field false even when the view was resolved with
standard semantics enabled. That can reject no-SerDe scalar output or let the
Hive path deserialize through bounded CHAR/VARCHAR types instead of enforcing
the saved mode.
See **Shared repair plan 1** in the review body.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +442,106 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including
nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip
children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType
match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType =
toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ /**
+ * Convert a Catalyst value from `sourceType` (JSON or unbounded STRING) to
Review Comment:
**Nit (P3):** The contract does not match the method below: sourceType is a
Catalyst DataType such as ArrayType, MapType, or StructType, while JSON is only
the serialized representation. Also, same-typed subtrees without CHAR/VARCHAR
return identity before the MapType branch, so unaffected maps are not always
rebuilt through ArrayBasedMapBuilder.
**Verification:**
- **Inspection:** Compare the revised source/target terminology and
map-rebuild claim with the method signature, early identity return, and MapType
branch, confirming every stated guarantee matches the implemented control flow.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveScriptTransformationExec.scala:
##########
@@ -268,6 +273,24 @@ object HiveScriptIOSchema extends HiveInspectors {
(columns, columnTypes)
}
+ /**
+ * Hive LazySimpleSerDe CHAR/VARCHAR types truncate on deserialize. Under
standard
+ * semantics, map them to STRING so Spark applies length checks. Only
LazySimpleSerDe
+ * and subclasses get this rewrite; other SerDes keep the declared schema.
+ * preserveCharVarcharTypeInfo without standard semantics is unchanged.
+ */
+ private def outputTypesForSerDe(
+ serdeClassName: String,
+ columnTypes: Seq[DataType]): Seq[DataType] = {
+ val serdeClass = Utils.classForName[AbstractSerDe](serdeClassName)
+ if (SQLConf.get.charVarcharStandardSemantics &&
Review Comment:
**Non-blocking (P2):** This re-reads the task's SQLConf even though the
script plan has already bound its CHAR/VARCHAR mode. If a plan is materialized
with standard semantics enabled and executed after the session disables it,
this branch passes bounded CHAR/VARCHAR types to LazySimpleSerDe, so Hive can
truncate overlength output before the typed unwrapper can raise
EXCEED_LIMIT_LENGTH. The reverse transition can also enable the rewrite for a
disabled-mode plan.
See **Shared repair plan 1** in the review body.
--
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]
