kosiew commented on code in PR #16696:
URL: https://github.com/apache/datafusion/pull/16696#discussion_r2241482599


##########
datafusion/optimizer/src/optimize_projections/mod.rs:
##########
@@ -826,6 +840,34 @@ pub fn is_projection_unnecessary(
     ))
 }
 
+fn plan_contains_subquery_alias(plan: &LogicalPlan) -> bool {
+    // ambiguity of aliases can arise if there are

Review Comment:
   ```rust
           LogicalPlan::RecursiveQuery(_) => {
               plan.inputs()
                   .into_iter()
                   .map(|input| {
                       indices
                           .clone()
                           .with_projection_beneficial()
                           .with_plan_exprs(&plan, input.schema())
                   })
                   .collect::<Result<Vec<_>>>()?
           }
   ```        
   in fn optimize_projectsion caused this slt test to fail
   ```
   External error: 1 errors in file 
/Users/kosiew/GitHub/datafusion/datafusion/sqllogictest/test_files/cte.slt
   
   1. query failed: DataFusion error: Optimizer rule 'optimize_projections' 
failed
   caused by
   Schema error: No field named __scalar_sq_1.prices_row_num. Did you mean 
'__scalar_sq_3.prices_row_num'?.
   [SQL] WITH RECURSIVE "recursive_cte" AS (
    ...
   
   ```
   ```sql
   # CTE within window function inside nested CTE works. This test demonstrates 
using a nested window function to recursively iterate over a column.
   query RRII
   WITH RECURSIVE "recursive_cte" AS (
     (
       WITH "min_prices_row_num_cte" AS (
      ...
   ```
   
   After a lot of investigation,  I found that it failed because the recursive 
cte has several subquery aliases.
   Instead of tweaking subquery aliases for recursive ctes,  I added a bypass 
for recursive ctes with 2 or more subquery aliases.
   ```rust
           LogicalPlan::RecursiveQuery(_) => {
               if plan_contains_subquery_alias(&plan) {
                   // 
https://github.com/apache/datafusion/pull/16696#discussion_r2241482599
                   return Ok(Transformed::no(plan));
               }
           ...  
   ```  
   which enabled the slt test to pass.
   
   I thought this was an improvement because before this change, recursive 
queries were not transformed by optimize_projections at all.
   
   



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to