Skip to content

[CALCITE-3004] RexOver is incorrectly pushed down in ProjectSetOpTran…#1166

Merged
hsyuan merged 1 commit intoapache:masterfrom
chunweilei:push_rexover_down_incorrectly
Apr 23, 2019
Merged

[CALCITE-3004] RexOver is incorrectly pushed down in ProjectSetOpTran…#1166
hsyuan merged 1 commit intoapache:masterfrom
chunweilei:push_rexover_down_incorrectly

Conversation

@chunweilei
Copy link
Contributor

…sposeRule and ProjectCorrelateTransposeRule (Chunwei Lei)

JIRA: https://issues.apache.org/jira/browse/CALCITE-3004

setOp.copy(setOp.getTraitSet(), newSetOpInputs);
node = pushProject.createNewProject(newSetOp, adjustments);
} else {
// push some expressions below the setop; this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is if..else.. in that I don't want to change the plan of some cases. For instance, current plan of

select ename,avg(empno) from 
(select * from emp as e1 union all select * from emp as e2)

is

LogicalAggregate(group=[{0}], EXPR$1=[AVG($1)])
  LogicalUnion(all=[true])
    LogicalProject(ENAME=[$1], EMPNO=[$0])
      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
    LogicalProject(ENAME=[$1], EMPNO=[$0])
      LogicalTableScan(table=[[CATALOG, SALES, EMP]])

But if without if..else.., the plan becomes

LogicalAggregate(group=[{0}], EXPR$1=[AVG($1)])
  LogicalProject(ENAME=[$1], EMPNO=[$0])
    LogicalUnion(all=[true])
      LogicalProject(EMPNO=[$0], ENAME=[$1])
        LogicalTableScan(table=[[CATALOG, SALES, EMP]])
      LogicalProject(EMPNO=[$0], ENAME=[$1])
        LogicalTableScan(table=[[CATALOG, SALES, EMP]])

which seems redundant.

</TestCase>
<TestCase name="testProjectCorrelateTranspose">
<TestCase name="testProjectCorrelateTransposeWithOver">
<Resource name="sql">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Somehow there's a duplicate key in RelOptRulesTest.xml. Maybe it's a merge error, I don't know. Can someone add code to DiffRepository to make it fail when reading a .xml file if there are duplicate keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julianhyde I would like to do it. : )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue(CALCITE-3009) for it.

@chunweilei
Copy link
Contributor Author

@julianhyde @zhztheplayer, could you please review the PR? Thanks a lot.

…sposeRule and ProjectCorrelateTransposeRule (Chunwei Lei)
@chunweilei chunweilei force-pushed the push_rexover_down_incorrectly branch from 995dbfe to d166bdb Compare April 23, 2019 05:45
@chunweilei
Copy link
Contributor Author

Updated for resolving conflicts.

Copy link
Member

@hsyuan hsyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@hsyuan hsyuan merged commit f26d92f into apache:master Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants