comphead opened a new pull request, #6574: URL: https://github.com/apache/datafusion-comet/pull/6574
## Which issue does this PR close? Closes #6572. ## Rationale for this change Spark's `InMemoryTableScanExec` lists its `InMemoryRelation` as an inner child, and the relation lists the cached plan, so `EXPLAIN` and `EXPLAIN FORMATTED` draw the plan that built the cache below the scan. `CometInMemoryTableScanExec` did not, so for a relation cached in Comet's format the tree ended at the scan. #6453 left `innerChildren` out on purpose: `ExtendedExplainInfo` walks it, so the cached plan's operators and fallback reasons would have counted toward every query that reads the cache. This PR adds the inner child and keeps it out of Comet's reporting. ## What changes are included in this PR? - `CometInMemoryTableScanExec.innerChildren` returns `Seq(originalPlan.relation)`, as Spark's scan does. `EXPLAIN` and `EXPLAIN FORMATTED` now draw and number the relation and the cached plan below the scan. - `ExtendedExplainInfo.executionInnerChildren` skips the scan's inner children, as it already does for `CometEmptyRelationExec`. The cached plan runs when the relation is materialized, not as part of every query that reads it. Spark's own `InMemoryTableScanExec` is unchanged, so its cached plan is still counted, as before. Worth calling out: - On Spark 3.4 and 3.5, `EXPLAIN FORMATTED` prints the relation's `CachedRDDBuilder` in the `InMemoryRelation` detail entry, as it does below Spark's own scan on those versions. [SPARK-51861](https://issues.apache.org/jira/browse/SPARK-51861) removed that in Spark 4.0. The tree itself is clean on every version. The test from #6453 now checks the whole `FORMATTED` output on Spark 4.0 and later, and only the scan's own detail entry on 3.x. - This does not fix #6463. The SQL tab's graph and the event log's plan tree come from `SparkPlanInfo.fromSparkPlan`, which does not read `innerChildren` in any supported Spark version. The text plan description (the SQL tab's Details and the event log's `physicalPlanDescription`) comes from `explainString`, so it should now show the cached plan, though that was not checked at runtime. ## How are these changes tested? A new test in `CometInMemoryCacheSuite` runs with AQE off and on. It checks that: - the relation's line and then the cached plan's lines follow the scan in the tree string - `EXPLAIN FORMATTED` numbers `InMemoryRelation` right below the scan - a fallback reason recorded in the cached plan stays out of the reading query's fallback reasons and out of Comet's verbose explain tree The test fails without the `innerChildren` override and without the `ExtendedExplainInfo` change. The test from #6453 is adjusted as described above. `CometInMemoryCacheSuite` passes locally on the default Spark 4.1 profile (60 tests). Other profiles were not run locally. Spark's SQL tests should be unaffected, because Comet's in-memory cache is disabled by default. -- 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]
