[ https://issues.apache.org/jira/browse/CALCITE-5715?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17726831#comment-17726831 ]
Julian Hyde commented on CALCITE-5715: -------------------------------------- I've been confused by the summaries of a few of your recent cases. (This, and CALCITE-5506 and CALCITE-5717.) When you use the word 'prune' I tend of think of pruning fields or RexNodes. Can we find a clearer terminology? Summaries are important, because that is what ends up in the release notes and becomes the documentation. Also, I personally refuse to review a PR until I understand its goal. The specification needs to be good enough that people don't need to read the implementation. Otherwise we are not doing software engineering. In the case of CALCITE-5717 I found it clearer to reference the before and after RelNode classes: "Replace Project on Aggregate with Values". Pruning a tree with N branches results in a tree with N-1 branches. So in this case, which removes a trivial Project, I wouldn't refer to the process as 'pruning'. > Prune old nodes after projection merging in ProjectMergeRule > ------------------------------------------------------------ > > Key: CALCITE-5715 > URL: https://issues.apache.org/jira/browse/CALCITE-5715 > Project: Calcite > Issue Type: Improvement > Components: core > Affects Versions: 1.34.0 > Reporter: Benchao Li > Priority: Major > Fix For: 1.35.0 > > > We already have many similar usages (prune old nodes when the new one is > obviously better, e.g. in > [CalcMergeRule|https://github.com/apache/calcite/blob/c56d5564628de6b3265f960764a6f6fc43935a75/core/src/main/java/org/apache/calcite/rel/rules/CalcMergeRule.java#L85-L90] > to reduce the search scope, I propose to also add this for > {{ProjectMergeRule}}. Do you have any concerns about this? > In {{ProjectMergeRule}}, there is a case that after merging the two > {{Project}}, we'll find the new Project is trivial and we'll transform to the > bottomProject's > input(https://github.com/apache/calcite/blob/c56d5564628de6b3265f960764a6f6fc43935a75/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java#L132-L139). > In this case, both topProject and bottomProject could be pruned? > The reason I propose to do this optimization for {{ProjectMergeRule}} is that > I met a problem that two sets will point to each other after projection > merging, and the planner will run into dead loop. (It's hard to show the > details, I haven't got a simple reproducible demo yet) -- This message was sent by Atlassian Jira (v8.20.10#820010)