Dandandan commented on a change in pull request #9776:
URL: https://github.com/apache/arrow/pull/9776#discussion_r600195534
##########
File path: rust/datafusion/src/sql/planner.rs
##########
@@ -103,17 +104,31 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
/// Generate a logic plan from an SQL query
pub fn query_to_plan(&self, query: &Query) -> Result<LogicalPlan> {
- self.query_to_plan_with_alias(query, None)
+ self.query_to_plan_with_alias(query, None, &mut HashMap::new())
}
/// Generate a logic plan from an SQL query with optional alias
pub fn query_to_plan_with_alias(
&self,
query: &Query,
alias: Option<String>,
+ ctes: &mut HashMap<String, LogicalPlan>,
) -> Result<LogicalPlan> {
let set_expr = &query.body;
- let plan = self.set_expr_to_plan(set_expr, alias)?;
+ if let Some(with) = &query.with {
+ // Process CTEs from top to bottom
+ // do not allow self-references
+ for cte in &with.cte_tables {
+ // create logical plan & pass backreferencing CTEs
+ let logical_plan = self.query_to_plan_with_alias(
+ &cte.query,
+ Some(cte.alias.name.value.clone()),
+ &mut ctes.clone(),
Review comment:
This clone is now relatively inefficient for very long and/or deep ctes.
Two (common) solutions I see for this problem.
* use an immutable HashMap (O(1) clone, easier to program), example:
https://docs.rs/im/15.0.0/im/struct.HashMap.html
* use something like "frames", e.g. `Vec<HashMap<String, LogicalPlan>>` ->
when looking up a reference first look up level 0, level-1, level -2, etc.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
