srielau commented on code in PR #59467:
URL: https://github.com/apache/spark/pull/59467#discussion_r4296060755
##########
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:
Fixed in `AnalyzedSchemaProjection.rebindToAnalyzedSchema`: cleaned vs
freshly stamped CHAR/VARCHAR metadata is treated as the same attribute shape,
so a padded versioned relation keeps the cleaned attribute instead of being
rebound to a stamped one. `toOutputAttrs` then drops the marker on the
optimizer path as well. Comments updated to match.
##########
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:
Went with preserving cleaned metadata in `rebindToAnalyzedSchema` rather
than restoring the conf disjunct. A padded versioned InMemory table now stays
unstamped through refresh, so the pruned scan output is a real regression
guard. Comments updated.
##########
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:
The extra matrix case is in, and with the rebind fix the padded prune tests
now also assert the optimized scan output has no marker (the InMemory refresh
path is no longer stamped).
--
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]
