peter-toth commented on code in PR #59996:
URL: https://github.com/apache/spark/pull/59996#discussion_r4443792649
##########
sql/core/src/test/scala/org/apache/spark/sql/DatasetOptimizationSuite.scala:
##########
@@ -132,6 +132,29 @@ class DatasetOptimizationSuite extends SharedSparkSession {
}
}
+ test("Nested projections bind to the fields emitted by the serializer") {
+ val ds = spark.range(1).map { _ =>
+ Tuple1((Map("k" -> ("map-a", "map-b")), ("other-a", "other-b")))
+ }
+ val query = ds.selectExpr("_1._1['k']._2", "_1._2._1")
+ // The map and its sibling share a serializer. Visiting the map converters
twice causes
+ // pruning to fall back to the original schema, so the projection must use
its ordinals.
+ assert(query.queryExecution.optimizedPlan.collectFirst {
+ case s: SerializeFromObject => s.schema
+ }.contains(ds.schema))
+ checkAnswer(query, Row("map-b", "other-a"))
+ }
+
+ test("Nested projections resolve case-insensitive references against the
serializer") {
+ withSQLConf(SQLConf.CASE_SENSITIVE.key -> "false") {
+ val input = Seq((Seq(("unused", "wanted")), 1)).toDF("Items", "Id")
+ val ds = input.map(row => row)(ExpressionEncoder(input.schema))
+ val query = ds.selectExpr("items[0]._2", "ITEMS._2", "iD")
+ checkAnswer(query, Row("wanted", Seq("wanted"), 1))
Review Comment:
**Finding 3.** Both spellings here select `_2`, so this test can't tell
whether `identifyRootFields` sees the normalized names. I changed
`objects.scala:235` to `SchemaPruning.identifyRootFields(p.projectList,
Seq.empty)`:
- both new tests still pass
- `selectExpr("s._1", "S._3")` over a struct column `S` fails with
`FIELD_NOT_FOUND`, as on base
- this test with `ITEMS._1` fails the same way
With the change below, the test passes here, fails on base
(`ArrayIndexOutOfBoundsException`), and fails with that mutation
(`FIELD_NOT_FOUND`):
```suggestion
val query = ds.selectExpr("items[0]._2", "ITEMS._1", "iD")
checkAnswer(query, Row("wanted", Seq("unused"), 1))
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/objects.scala:
##########
@@ -221,25 +222,36 @@ object ObjectSerializerPruning extends Rule[LogicalPlan] {
val usedRefs = p.references
val prunedSerializer = s.serializer.filter(usedRefs.contains)
- val rootFields = SchemaPruning.identifyRootFields(p.projectList,
Seq.empty)
-
- if (conf.serializerNestedSchemaPruningEnabled && rootFields.nonEmpty) {
- // Prunes nested fields in serializers.
- val prunedSchema = SchemaPruning.pruneSchema(
- DataTypeUtils.fromAttributes(prunedSerializer.map(_.toAttribute)),
rootFields)
- val nestedPrunedSerializer = prunedSerializer.zipWithIndex.map { case
(serializer, idx) =>
- pruneSerializer(serializer, prunedSchema(idx).dataType)
- }
-
- // Builds new projection.
- val projectionOverSchema = ProjectionOverSchema(prunedSchema,
AttributeSet(s.output))
- val newProjects = p.projectList.map(_.transformDown {
- case projectionOverSchema(expr) => expr
- }).map { case expr: NamedExpression => expr }
- p.copy(projectList = newProjects,
- child = SerializeFromObject(nestedPrunedSerializer, s.child))
- } else {
+ if (!conf.serializerNestedSchemaPruningEnabled) {
p.copy(child = SerializeFromObject(prunedSerializer, s.child))
+ } else {
+ // Normalize the root name so references such as items and ITEMS do
not become
+ // distinct fields in pruneSchema during case-insensitive analysis.
Review Comment:
**Finding 4.** Minor: the normalized names also feed the rebuild below.
`items[0]._2` alone has one root, so `pruneSchema` handles it fine. It fails on
base because `ProjectionOverSchema` looks up `a.name` by exact name
(`ProjectionOverSchema.scala:40`) and leaves the access unchanged. This keeps
the concrete case from
[r4437932629](https://github.com/apache/spark/pull/59996#discussion_r4437932629)
and adds the second use:
```suggestion
// Use the serializer's names for references to its output.
Otherwise:
// - pruneSchema treats items and ITEMS as distinct roots.
// - ProjectionOverSchema leaves an access through items unchanged,
since it looks up
// Items by exact name.
```
--
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]
