You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/06/01 21:24:34 UTC
[GitHub] [calcite] scrozon opened a new pull request #1998: [CALCITE-4037] AggregateProjectMergeRule doesn't always respect aliases
scrozon opened a new pull request #1998:
URL: https://github.com/apache/calcite/pull/1998
adds a project on top of the aggregate when the field names of the
new aggregate don't match the field names of the original one
The side effects are mostly an extra project node in some of the existing unit tests, which I believe is correct since the existing tests were not respecting the aliases.
The only unit test where my changes impacts the structure of the plan is `testPullAggregateThroughUnionWithAlias` where the original version removed the aggregate node in the 2nd input of the union, but I'm not sure why that was the case, both inputs should be aggregated before the union I think.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [calcite] diegotremper commented on pull request #1998: [CALCITE-4037] AggregateProjectMergeRule doesn't always respect aliases
Posted by GitBox <gi...@apache.org>.
diegotremper commented on pull request #1998:
URL: https://github.com/apache/calcite/pull/1998#issuecomment-866271028
hey guys, do you intend to merge this pull request?
We encountered this issue in version 1.25, after applying this fix the problem was fixed.
Thanks!
--
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:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #1998: [CALCITE-4037] AggregateProjectMergeRule doesn't always respect aliases
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #1998:
URL: https://github.com/apache/calcite/pull/1998#discussion_r464216262
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectMergeRule.java
##########
@@ -128,7 +131,7 @@ public static RelNode apply(RelOptRuleCall call, Aggregate aggregate,
i < newAggregate.getRowType().getFieldCount(); i++) {
posList.add(i);
}
- relBuilder.project(relBuilder.fields(posList));
+ relBuilder.projectNamed(relBuilder.fields(posList), originalFieldNames, true);
Review comment:
Would the added project be removed by `ProjectRemoveRule`?
----------------------------------------------------------------
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:
users@infra.apache.org