You say "fix this to make it work again” but I’m not sure it ever worked the way you want it to. Note that that block was added in https://issues.apache.org/jira/browse/CALCITE-92 <https://issues.apache.org/jira/browse/CALCITE-92>, which I think you would agree was a change for the better.
I’ve no objection to you looking for Projects inside a RelSubset. Remember that there might be several. You could also check for HepRelVertex. Julian > On Jan 18, 2018, at 5:57 PM, Shuyi Chen <[email protected]> wrote: > > Thanks a lot, Julian. I was actually looking at the this code block in > ProjectRemoveRule, it's actually trying to preserve the rename info of the > top project (see the if block), so it may actually produce something like > the following: > > LogicalAggregate(group=[{0}]) > LogicalProject(departmentNo=[$7]) > LogicalTableScan(table=[[scott, EMP]]) > > public void onMatch(RelOptRuleCall call) { > Project project = call.rel(0); > assert isTrivial(project); > RelNode stripped = project.getInput(); > if (stripped instanceof Project) { > // Rename columns of child projection if desired field names are given. > Project childProject = (Project) stripped; > stripped = childProject.copy(childProject.getTraitSet(), > childProject.getInput(), childProject.getProjects(), > project.getRowType()); > } > RelNode child = call.getPlanner().register(stripped, project); > call.transformTo(child); > } > > > However, "stripped" is a RelSubset which contains a Project during the > volcano planner optimization. So the if () code block won't be executed. Do > you think we can fix this to make it work again? Thanks a lot. > > Shuyi > > > On Thu, Jan 18, 2018 at 3:54 PM, Julian Hyde <[email protected]> wrote: > >> Planner rules do their best to keep field names the same, but they don’t >> guarantee it. If they did, they’d miss a lot of good opportunities to >> optimize. >> >> By the way, the same goes for RelBuilder. >> >> Julian >> >> >>> On Jan 18, 2018, at 3:50 PM, Shuyi Chen <[email protected]> wrote: >>> >>> Hi, >>> >>> I have a question regarding the ProjectRemoveRule. In Calcite 1980, a >> test >>> case was introduced below: >>> >>> /** Test case for >>> * <a href="https://issues.apache.org/jira/browse/CALCITE-1980"> >> [CALCITE-1980] >>> * RelBuilder gives NPE if groupKey contains alias</a>. >>> * >>> * <p>Now, the alias does not cause a new expression to be added to the >> input, >>> * but causes the referenced fields to be renamed. */ >>> @Test public void testAggregateProjectWithAliases() { >>> final RelBuilder builder = RelBuilder.create(config().build()); >>> RelNode root = >>> builder.scan("EMP") >>> .project(builder.field("DEPTNO")) >>> .aggregate( >>> builder.groupKey( >>> builder.alias(builder.field("DEPTNO"), >> "departmentNo"))) >>> .build(); >>> final String expected = "" >>> + "LogicalAggregate(group=[{0}])\n" >>> + " LogicalProject(departmentNo=[$0])\n" >>> + " LogicalProject(DEPTNO=[$7])\n" >>> + " LogicalTableScan(table=[[scott, EMP]])\n"; >>> assertThat(str(root), is(expected)); >>> } >>> >>> So according the ProjectRemoveRule, which will remove the top project, >> the >>> above REL will be optimized as the following: >>> >>> LogicalAggregate(group=[{0}]) >>> LogicalProject(DEPTNO=[$7]) >>> LogicalTableScan(table=[[scott, EMP]]) >>> >>> But this will lose rename information "departmentNo". Is this expected, >> and >>> is it a problem that should get fixed? Please correct me if I am wrong. >>> Thanks a lot. >>> >>> Shuyi >>> -- >>> "So you have to trust that the dots will somehow connect in your future." >> >> > > > -- > "So you have to trust that the dots will somehow connect in your future."
