You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/15 15:29:05 UTC

[GitHub] [arrow-datafusion] jackwener opened a new pull request, #2244: Replace Union.alias with SubqueryAlias

jackwener opened a new pull request, #2244:
URL: https://github.com/apache/arrow-datafusion/pull/2244

   # Which issue does this PR close?
   
   Closes #2213.
   
   # What changes are included in this PR?
   
   Replace Union.alias with SubqueryAlias
   
   # Are there any user-facing changes?
   None
   


-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-datafusion] jackwener commented on pull request #2244: Replace Union.alias with SubqueryAlias

Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #2244:
URL: https://github.com/apache/arrow-datafusion/pull/2244#issuecomment-1100220270

   > Error: Plan("No field named 'all.cat'. Valid fields are 'all.i'.")
   
   OMG, this error is so hard to debug. I often meetup the schema match problem like #2084 . I think this isn't a good design......
   
   


-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-datafusion] jackwener commented on a diff in pull request #2244: Replace Union.alias with SubqueryAlias

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #2244:
URL: https://github.com/apache/arrow-datafusion/pull/2244#discussion_r851451355


##########
datafusion/core/src/optimizer/projection_push_down.rs:
##########
@@ -455,6 +450,33 @@ fn optimize_plan(
                     let expr = vec![];
                     utils::from_plan(plan, &expr, &new_inputs)
                 }
+                LogicalPlan::Union(Union { inputs, .. }) => {
+                    // let new_required_columns = new_required_columns
+                    //     .iter()
+                    //     .map(|c| match &c.relation {
+                    //         Some(q) if q == alias => Column {
+                    //             relation: Some(table_name.clone()),
+                    //             name: c.name.clone(),
+                    //         },
+                    //         _ => c.clone(),
+                    //     })
+                    //     .collect();
+
+                    let new_inputs = inputs
+                        .iter()
+                        .map(|input_plan| {
+                            optimize_plan(
+                                _optimizer,
+                                input_plan,
+                                &new_required_columns,
+                                has_projection,
+                                _execution_props,
+                            )
+                        })
+                        .collect::<Result<Vec<_>>>()?;
+                    let expr = vec![];
+                    utils::from_plan(plan, &expr, &new_inputs)
+                }

Review Comment:
   This rewrite is wrong



-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-datafusion] jackwener closed pull request #2244: Replace Union.alias with SubqueryAlias

Posted by GitBox <gi...@apache.org>.
jackwener closed pull request #2244: Replace Union.alias with SubqueryAlias
URL: https://github.com/apache/arrow-datafusion/pull/2244


-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-datafusion] jackwener commented on a diff in pull request #2244: Replace Union.alias with SubqueryAlias

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #2244:
URL: https://github.com/apache/arrow-datafusion/pull/2244#discussion_r851451355


##########
datafusion/core/src/optimizer/projection_push_down.rs:
##########
@@ -455,6 +450,33 @@ fn optimize_plan(
                     let expr = vec![];
                     utils::from_plan(plan, &expr, &new_inputs)
                 }
+                LogicalPlan::Union(Union { inputs, .. }) => {
+                    // let new_required_columns = new_required_columns
+                    //     .iter()
+                    //     .map(|c| match &c.relation {
+                    //         Some(q) if q == alias => Column {
+                    //             relation: Some(table_name.clone()),
+                    //             name: c.name.clone(),
+                    //         },
+                    //         _ => c.clone(),
+                    //     })
+                    //     .collect();
+
+                    let new_inputs = inputs
+                        .iter()
+                        .map(|input_plan| {
+                            optimize_plan(
+                                _optimizer,
+                                input_plan,
+                                &new_required_columns,
+                                has_projection,
+                                _execution_props,
+                            )
+                        })
+                        .collect::<Result<Vec<_>>>()?;
+                    let expr = vec![];
+                    utils::from_plan(plan, &expr, &new_inputs)
+                }

Review Comment:
   This rewrite is wrong



-- 
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-unsubscribe@arrow.apache.org

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