jayzhan211 commented on code in PR #26271:
URL: https://github.com/apache/datafusion/pull/26271#discussion_r4370123587
##########
datafusion/core/src/physical_planner.rs:
##########
@@ -3311,6 +3387,7 @@ impl DefaultPhysicalPlanner {
where
F: FnMut(&dyn ExecutionPlan, &dyn PhysicalOptimizerRule),
{
+ let plan = bind_materialized_cte_scans(plan, session_state)?;
Review Comment:
Since the scans now carry exact statistics, `AggregateStatistics` often
folds a `count(*)` over a scan into a constant and drops the scan. That scan
was already bound here, though, and `bind` pushed its partition count into
`scan_partitions`. So the replay reservation still includes readers that never
run. With `target_partitions = 1` and `memory_limit = '300K'`, this fails while
asking for 576 KB, nine scans' worth, even though only the `sum` scan is left:
```sql
WITH g AS MATERIALIZED (SELECT value AS x FROM generate_series(1, 200000))
SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g UNION ALL SELECT
count(*) FROM g
UNION ALL SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g UNION ALL
SELECT count(*) FROM g
UNION ALL SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g UNION ALL
SELECT sum(x) FROM g;
```
Main reserves the same amount for this query, so it isn't a regression, and
a follow-up is fine. Could the partition counts be registered from the scans in
the final plan instead, for example after the optimizers or when
`MaterializedCteExec::execute` sets the body?
--
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]
