peter-toth commented on code in PR #59996:
URL: https://github.com/apache/spark/pull/59996#discussion_r4436903926
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/objects.scala:
##########
@@ -232,12 +242,13 @@ object ObjectSerializerPruning extends Rule[LogicalPlan] {
}
// Builds new projection.
- val projectionOverSchema = ProjectionOverSchema(prunedSchema,
AttributeSet(s.output))
- val newProjects = p.projectList.map(_.transformDown {
+ val serialized = SerializeFromObject(nestedPrunedSerializer, s.child)
+ val projectionOverSchema = ProjectionOverSchema(serialized.schema,
AttributeSet(s.output))
Review Comment:
**Finding 1.** With this change the regression query reads the right field,
but its serializer is still not pruned. On this head, `SerializeFromObject` for
`ds.selectExpr("_1._1['k']._2", "_1._2._1")` emits the full
`struct<_1:struct<_1:map<string,struct<_1:string,_2:string>>,_2:struct<_1:string,_2:string>>>`.
The fallback comes from `pruneSerializer` (`objects.scala:195-208`):
- the `ExternalMapToCatalyst` case prunes the map value struct with the next
type from `prunedStructTypes`;
- `transformDown` then visits the converters again, since they are children
of `ExternalMapToCatalyst`;
- that second visit takes the type meant for the sibling `_2` struct, so the
result no longer matches `prunedDataType`, and the original serializer is
returned.
A recursion that visits each node once fixes it:
```scala
def prune(expr: Expression): Expression = expr match {
case m: ExternalMapToCatalyst =>
m.copy(
keyConverter = alignNullTypeInIf(prune(m.keyConverter)),
valueConverter = alignNullTypeInIf(prune(m.valueConverter)),
inputData = prune(m.inputData))
case s: CreateNamedStruct if prunedStructTypes.hasNext =>
pruneNamedStruct(s, prunedStructTypes.next()).mapChildren(prune)
case other => other.mapChildren(prune)
}
```
With it, the same query prunes to
`struct<_1:struct<_1:map<string,struct<_2:string>>,_2:struct<_1:string>>>` and
still returns `[map-b,other-a]`. `DatasetOptimizationSuite` and
`ObjectSerializerPruningSuite` pass. Base doesn't prune this query either, so
it's not a regression, and it could go in here or in a follow-up ticket.
Rebuilding against `serialized.schema` stays right either way, since
`pruneSerializer` can still fall back.
--
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]
