comphead opened a new issue, #6572:
URL: https://github.com/apache/datafusion-comet/issues/6572
### Describe the bug
Spark's cache scan draws the cached relation and the plan that built it
below itself in `EXPLAIN` and `EXPLAIN FORMATTED`. Both come from
`innerChildren`:
```scala
// InMemoryTableScanExec
override def innerChildren: Seq[QueryPlan[_]] = Seq(relation) ++
super.innerChildren
// InMemoryRelation
override def innerChildren: Seq[SparkPlan] = Seq(cachedPlan)
```
`CometInMemoryTableScanExec` does not override `innerChildren`, so for a
relation cached in Comet's format the tree ends at the scan. The
`InMemoryRelation` line, with its storage level, and the cached plan are both
missing.
### Steps to reproduce
Start with `spark.comet.exec.inMemoryCache.enabled=true`, cache a relation
and print `EXPLAIN FORMATTED` for a query whose plan uses
`CometInMemoryTableScan`. An aggregate over a cached table on Spark 4.x printed
this tree (statistics removed):
```
AdaptiveSparkPlan (12)
+- == Final Plan ==
ResultQueryStage (8)
+- * CometColumnarToRow (7)
+- CometHashAggregate (6)
+- ShuffleQueryStage (5)
+- CometExchange (4)
+- CometHashAggregate (3)
+- TableCacheQueryStage (2)
+- CometInMemoryTableScan (1)
```
Nothing is drawn below `CometInMemoryTableScan (1)`. The IDs show the same
thing: `ExplainUtils.generateOperatorIDs` numbers a node's inner children right
after the node, and here the next ID goes to `TableCacheQueryStage (2)`.
### Expected behavior
The scan is followed by its `InMemoryRelation` and the cached plan, as
Spark's `InMemoryTableScan` is, in both `EXPLAIN` and `EXPLAIN FORMATTED`.
### Additional context
#6453 left `innerChildren` out on purpose. `ExtendedExplainInfo` walks
`innerChildren`, so the cached plan's operators and fallback reasons would
count toward the coverage stats and fallback warnings of every query that reads
the cache.
`CometEmptyRelationExec` already handles this situation. It keeps its
subtree in `innerChildren` for Spark's tree string, and
`ExtendedExplainInfo.executionInnerChildren` leaves it out of Comet's
reporting. The same approach could work here. Untested sketch:
```scala
// CometInMemoryTableScanExec
override def innerChildren: Seq[QueryPlan[_]] = Seq(originalPlan.relation)
// ExtendedExplainInfo.executionInnerChildren
case _: CometEmptyRelationExec | _: CometInMemoryTableScanExec => Seq.empty
```
One open question: when Spark's own `InMemoryTableScanExec` reads a cache,
`ExtendedExplainInfo` already counts its cached plan today. With the sketch
above, the two scans would report differently. Should Spark's scan be excluded
too?
The SQL tab and event log lose the cached plan for a different reason,
covered by #6463. Overriding `innerChildren` does not change that.
Code references are to `main` at f980d6fb59.
--
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]
