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."

Reply via email to