srielau commented on code in PR #59467:
URL: https://github.com/apache/spark/pull/59467#discussion_r4295951695
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala:
##########
@@ -717,9 +717,21 @@ object PushDownUtils extends Logging {
relation: DataSourceV2Relation): Seq[AttributeReference] = {
val nameToAttr = Utils.toMap(relation.output.map(_.name), relation.output)
val cleaned = CharVarcharUtils.replaceCharVarcharWithStringInSchema(schema)
- toAttributes(cleaned).map {
- // we have to keep the attribute id during transformation
- a => a.withExprId(nameToAttr(a.name).exprId)
+ toAttributes(cleaned).map { a =>
+ val orig = nameToAttr(a.name)
+ // Keep exprId. Do not re-stamp __CHAR_VARCHAR_TYPE_STRING after
+ // ApplyCharTypePadding cleaned it: that marker means "not yet padded",
+ // and this runs in the optimizer with no second padding pass. File
+ // sources usually already substituted STRING, so the stamp was latent;
+ // a V2 scan whose readSchema() still surfaces CHAR/VARCHAR would
+ // otherwise look unpadded. Key off the relation's own padding state,
+ // not the session conf: a still-stamped relation was not padded.
Review Comment:
Reworded to the structural rule the code applies: drop the marker only when
the relation attribute already has none, keep it otherwise. That includes
versioned tables rebound by QueryExecution refresh after padding, which arrive
stamped again.
Left that versioned path as-is: no restored `charVarcharStandardSemantics`
disjunct, and no change to `rebindToAnalyzedSchema`. Happy to follow up
separately if we want to preserve cleaned metadata across the refresh.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala:
##########
@@ -717,9 +717,21 @@ object PushDownUtils extends Logging {
relation: DataSourceV2Relation): Seq[AttributeReference] = {
val nameToAttr = Utils.toMap(relation.output.map(_.name), relation.output)
val cleaned = CharVarcharUtils.replaceCharVarcharWithStringInSchema(schema)
- toAttributes(cleaned).map {
- // we have to keep the attribute id during transformation
- a => a.withExprId(nameToAttr(a.name).exprId)
+ toAttributes(cleaned).map { a =>
+ val orig = nameToAttr(a.name)
+ // Keep exprId. Do not re-stamp __CHAR_VARCHAR_TYPE_STRING after
+ // ApplyCharTypePadding cleaned it: that marker means "not yet padded",
+ // and this runs in the optimizer with no second padding pass. File
+ // sources usually already substituted STRING, so the stamp was latent;
+ // a V2 scan whose readSchema() still surfaces CHAR/VARCHAR would
+ // otherwise look unpadded. Key off the relation's own padding state,
+ // not the session conf: a still-stamped relation was not padded.
Review Comment:
Reworded both comments the same way: versioned InMemory tables are rebound
after padding and arrive stamped, so the marker is kept on that path. Same
scope decision as the other thread: no behavior change on the refresh rebind in
this PR.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -3999,4 +4085,78 @@ class DSV2CharVarcharTestSuite extends
CharVarcharTestSuite
}
}
}
+
+ private def withPrunedV2CharRelation(body: (DataFrame, DataSourceV2Relation)
=> Unit): Unit = {
+ withTable("std_v2_prune") {
+ sql(s"CREATE TABLE std_v2_prune (c CHAR(5), extra INT) USING $format")
+ sql("INSERT INTO std_v2_prune VALUES ('ab', 1)")
+ val df = sql("SELECT c FROM std_v2_prune")
+ val rel = df.queryExecution.analyzed.collect {
+ case r: DataSourceV2Relation => r
+ }.head
+ body(df, rel)
+ }
+ }
+
+ private def attrsFromCharReadSchema(rel: DataSourceV2Relation) =
+ PushDownUtils.toOutputAttrs(new StructType().add("c", CharType(5)), rel)
+
+ test("SPARK-59016: V2 column prune does not re-stamp CHAR padding
breadcrumb") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
Review Comment:
Added a case with standard semantics on and `READ_SIDE_CHAR_PADDING=false`.
`ApplyCharTypePadding` still pads, so the analyzed relation is cleaned, the
direct `toOutputAttrs` call drops the marker, and the query returns the padded
value. A gate keyed only on `readSideCharPadding` would fail this case.
--
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]
