alamb commented on code in PR #2334:
URL: https://github.com/apache/arrow-datafusion/pull/2334#discussion_r858034480
##########
datafusion/core/src/execution/context.rs:
##########
@@ -3243,7 +3243,7 @@ mod tests {
"SELECT cat, SUM(i) AS total FROM (
SELECT i, 'a' AS cat FROM catalog_a.schema_a.table_a
UNION ALL
- SELECT i, 'b' AS cat FROM catalog_b.schema_b.table_b
Review Comment:
I would recommend leaving this test alone and instead adding a new test,
perhaps in
https://github.com/apache/arrow-datafusion/blob/master/datafusion/core/tests/sql/union.rs
using the reproducer from
https://github.com/apache/arrow-datafusion/issues/2083
That way it is clear we haven't changed the coverage of this test and there
is a more focused tests for just this particular change
##########
datafusion/core/src/logical_plan/builder.rs:
##########
@@ -1055,19 +1055,23 @@ pub fn union_with_alias(
right_plan: LogicalPlan,
alias: Option<String>,
) -> Result<LogicalPlan> {
- let inputs = vec![left_plan, right_plan]
+ let union_schema = left_plan.schema().clone();
+ let inputs_iter = vec![left_plan, right_plan]
Review Comment:
I wonder if there is a reason you needed to make changes below this line
(other than to use `union_schema` rather than `schema` when creating the
`Projection`
In particular, I wonder if we can avoid the extra call to `clone()` by going
back to the previous setup?
--
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]
