nathanb9 commented on code in PR #25580:
URL: https://github.com/apache/datafusion/pull/25580#discussion_r4189904331
##########
datafusion/core/src/physical_planner.rs:
##########
@@ -211,14 +214,143 @@ impl DefaultPhysicalPlanner {
{
return Ok(plan);
}
+ let renumbered = renumber_duplicate_materialized_ctes(logical_plan)?;
+ let logical_plan = renumbered.as_ref().unwrap_or(logical_plan);
let plan = self
.create_initial_plan(logical_plan, session_state)
.await?;
+ let plan = bind_materialized_cte_scans(plan)?;
self.optimize_physical_plan(plan, session_state, |_, _| {})
}
}
+/// Give each occurrence of a [`MaterializedCte`] in `plan` its own id.
+///
+/// The id is copied when a logical plan is cloned, so one CTE can occur more
Review Comment:
I think there's a problem when a materialized CTE comes from a table that
plans its own saved query in `scan()`, like a `ViewTable` that isn't inlined.
Each read of that table plans the saved query separately, and the saved plan
always has the same CTE id. So reading it twice gives two `MaterializedCteExec`
nodes with the same id, and the final binding pass fails:
```sql
-- w is a view over `WITH r AS MATERIALIZED (SELECT a FROM t) SELECT a FROM
r`
SELECT * FROM w x, w y;
-- Internal error: Two MaterializedCteExec nodes have the id 1
```
It works with `NOT MATERIALIZED`. Could each planning run assign fresh ids?
--
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]
