This is an automated email from the ASF dual-hosted git repository. rubenql pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push: new 8e7d735d [CALCITE-4437] The Sort rel should be decorrelated even though it has fetch or limit when it is not inside a Correlate (Thomas Rebele) 8e7d735d is described below commit 8e7d735da428f9b193e86333ebe11712340dfeda Author: Thomas Rebele <thomas.reb...@gmail.com> AuthorDate: Wed Dec 16 11:37:39 2020 +0100 [CALCITE-4437] The Sort rel should be decorrelated even though it has fetch or limit when it is not inside a Correlate (Thomas Rebele) --- .../apache/calcite/sql2rel/RelDecorrelator.java | 75 +++++++++++----------- .../apache/calcite/test/SqlToRelConverterTest.java | 20 ++++++ .../apache/calcite/test/SqlToRelConverterTest.xml | 27 ++++++++ 3 files changed, 85 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java index 87f0adf..2fe9bd0 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -160,7 +160,8 @@ public class RelDecorrelator implements ReflectiveVisitor { protected final ReflectUtil.MethodDispatcher<@Nullable Frame> dispatcher = ReflectUtil.<RelNode, @Nullable Frame>createMethodDispatcher( Frame.class, getVisitor(), "decorrelateRel", - RelNode.class); + RelNode.class, + boolean.class); // The rel which is being visited protected @Nullable RelNode currentRel; @@ -288,7 +289,7 @@ public class RelDecorrelator implements ReflectiveVisitor { // Perform decorrelation. map.clear(); - final Frame frame = getInvoke(root, null); + final Frame frame = getInvoke(root, false, null); if (frame != null) { // has been rewritten; apply rules post-decorrelation final HepProgramBuilder builder = HepProgram.builder() @@ -401,14 +402,14 @@ public class RelDecorrelator implements ReflectiveVisitor { } /** Fallback if none of the other {@code decorrelateRel} methods match. */ - public @Nullable Frame decorrelateRel(RelNode rel) { + public @Nullable Frame decorrelateRel(RelNode rel, boolean isCorVarDefined) { RelNode newRel = rel.copy(rel.getTraitSet(), rel.getInputs()); if (rel.getInputs().size() > 0) { List<RelNode> oldInputs = rel.getInputs(); List<RelNode> newInputs = new ArrayList<>(); for (int i = 0; i < oldInputs.size(); ++i) { - final Frame frame = getInvoke(oldInputs.get(i), rel); + final Frame frame = getInvoke(oldInputs.get(i), isCorVarDefined, rel); if (frame == null || !frame.corDefOutputs.isEmpty()) { // if input is not rewritten, or if it produces correlated // variables, terminate rewrite @@ -429,7 +430,7 @@ public class RelDecorrelator implements ReflectiveVisitor { ImmutableSortedMap.of()); } - public @Nullable Frame decorrelateRel(Sort rel) { + public @Nullable Frame decorrelateRel(Sort rel, boolean isCorVarDefined) { // // Rewrite logic: // @@ -446,7 +447,7 @@ public class RelDecorrelator implements ReflectiveVisitor { // need to call propagateExpr. final RelNode oldInput = rel.getInput(); - final Frame frame = getInvoke(oldInput, rel); + final Frame frame = getInvoke(oldInput, isCorVarDefined, rel); if (frame == null) { // If input has not been rewritten, do not rewrite this rel. return null; @@ -474,16 +475,16 @@ public class RelDecorrelator implements ReflectiveVisitor { return register(rel, newSort, frame.oldToNewOutputs, frame.corDefOutputs); } - public @Nullable Frame decorrelateRel(Values rel) { + public @Nullable Frame decorrelateRel(Values rel, boolean isCorVarDefined) { // There are no inputs, so rel does not need to be changed. return null; } - public @Nullable Frame decorrelateRel(LogicalAggregate rel) { - return decorrelateRel((Aggregate) rel); + public @Nullable Frame decorrelateRel(LogicalAggregate rel, boolean isCorVarDefined) { + return decorrelateRel((Aggregate) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(Aggregate rel) { + public @Nullable Frame decorrelateRel(Aggregate rel, boolean isCorVarDefined) { // // Rewrite logic: // @@ -497,7 +498,7 @@ public class RelDecorrelator implements ReflectiveVisitor { assert !cm.mapRefRelToCorRef.containsKey(rel); final RelNode oldInput = rel.getInput(); - final Frame frame = getInvoke(oldInput, rel); + final Frame frame = getInvoke(oldInput, isCorVarDefined, rel); if (frame == null) { // If input has not been rewritten, do not rewrite this rel. return null; @@ -692,10 +693,10 @@ public class RelDecorrelator implements ReflectiveVisitor { } } - public @Nullable Frame getInvoke(RelNode r, @Nullable RelNode parent) { - final Frame frame = dispatcher.invoke(r); + public @Nullable Frame getInvoke(RelNode r, boolean isCorVarDefined, @Nullable RelNode parent) { + final Frame frame = dispatcher.invoke(r, isCorVarDefined); currentRel = parent; - if (frame != null && parent != null && r instanceof Sort) { + if (frame != null && isCorVarDefined && r instanceof Sort) { final Sort sort = (Sort) r; // Can not decorrelate if the sort has per-correlate-key attributes like // offset or fetch limit, because these attributes scope would change to @@ -723,11 +724,11 @@ public class RelDecorrelator implements ReflectiveVisitor { return null; } - public @Nullable Frame decorrelateRel(LogicalProject rel) { - return decorrelateRel((Project) rel); + public @Nullable Frame decorrelateRel(LogicalProject rel, boolean isCorVarDefined) { + return decorrelateRel((Project) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(Project rel) { + public @Nullable Frame decorrelateRel(Project rel, boolean isCorVarDefined) { // // Rewrite logic: // @@ -735,7 +736,7 @@ public class RelDecorrelator implements ReflectiveVisitor { // final RelNode oldInput = rel.getInput(); - Frame frame = getInvoke(oldInput, rel); + Frame frame = getInvoke(oldInput, isCorVarDefined, rel); if (frame == null) { // If input has not been rewritten, do not rewrite this rel. return null; @@ -1089,25 +1090,25 @@ public class RelDecorrelator implements ReflectiveVisitor { && type.getPrecision() >= type1.getPrecision(); } - public @Nullable Frame decorrelateRel(LogicalSnapshot rel) { + public @Nullable Frame decorrelateRel(LogicalSnapshot rel, boolean isCorVarDefined) { if (RexUtil.containsCorrelation(rel.getPeriod())) { return null; } - return decorrelateRel((RelNode) rel); + return decorrelateRel((RelNode) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(LogicalTableFunctionScan rel) { + public @Nullable Frame decorrelateRel(LogicalTableFunctionScan rel, boolean isCorVarDefined) { if (RexUtil.containsCorrelation(rel.getCall())) { return null; } - return decorrelateRel((RelNode) rel); + return decorrelateRel((RelNode) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(LogicalFilter rel) { - return decorrelateRel((Filter) rel); + public @Nullable Frame decorrelateRel(LogicalFilter rel, boolean isCorVarDefined) { + return decorrelateRel((Filter) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(Filter rel) { + public @Nullable Frame decorrelateRel(Filter rel, boolean isCorVarDefined) { // // Rewrite logic: // @@ -1125,7 +1126,7 @@ public class RelDecorrelator implements ReflectiveVisitor { // final RelNode oldInput = rel.getInput(); - Frame frame = getInvoke(oldInput, rel); + Frame frame = getInvoke(oldInput, isCorVarDefined, rel); if (frame == null) { // If input has not been rewritten, do not rewrite this rel. return null; @@ -1156,11 +1157,11 @@ public class RelDecorrelator implements ReflectiveVisitor { frame.corDefOutputs); } - public @Nullable Frame decorrelateRel(LogicalCorrelate rel) { - return decorrelateRel((Correlate) rel); + public @Nullable Frame decorrelateRel(LogicalCorrelate rel, boolean isCorVarDefined) { + return decorrelateRel((Correlate) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(Correlate rel) { + public @Nullable Frame decorrelateRel(Correlate rel, boolean isCorVarDefined) { // // Rewrite logic: // @@ -1174,8 +1175,8 @@ public class RelDecorrelator implements ReflectiveVisitor { final RelNode oldLeft = rel.getInput(0); final RelNode oldRight = rel.getInput(1); - final Frame leftFrame = getInvoke(oldLeft, rel); - final Frame rightFrame = getInvoke(oldRight, rel); + final Frame leftFrame = getInvoke(oldLeft, isCorVarDefined, rel); + final Frame rightFrame = getInvoke(oldRight, true, rel); if (leftFrame == null || rightFrame == null) { // If any input has not been rewritten, do not rewrite this rel. @@ -1258,16 +1259,16 @@ public class RelDecorrelator implements ReflectiveVisitor { return register(rel, newJoin, mapOldToNewOutputs, corDefOutputs); } - public @Nullable Frame decorrelateRel(LogicalJoin rel) { - return decorrelateRel((Join) rel); + public @Nullable Frame decorrelateRel(LogicalJoin rel, boolean isCorVarDefined) { + return decorrelateRel((Join) rel, isCorVarDefined); } - public @Nullable Frame decorrelateRel(Join rel) { + public @Nullable Frame decorrelateRel(Join rel, boolean isCorVarDefined) { // For SEMI/ANTI join decorrelate it's input directly, // because the correlate variables can only be propagated from // the left side, which is not supported yet. if (!rel.getJoinType().projectsRight()) { - return decorrelateRel((RelNode) rel); + return decorrelateRel((RelNode) rel, isCorVarDefined); } // // Rewrite logic: @@ -1279,8 +1280,8 @@ public class RelDecorrelator implements ReflectiveVisitor { final RelNode oldLeft = rel.getInput(0); final RelNode oldRight = rel.getInput(1); - final Frame leftFrame = getInvoke(oldLeft, rel); - final Frame rightFrame = getInvoke(oldRight, rel); + final Frame leftFrame = getInvoke(oldLeft, isCorVarDefined, rel); + final Frame rightFrame = getInvoke(oldRight, isCorVarDefined, rel); if (leftFrame == null || rightFrame == null) { // If any input has not been rewritten, do not rewrite this rel. diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java index 1e9515e..a6c2e6a 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -1300,6 +1300,26 @@ class SqlToRelConverterTest extends SqlToRelTestBase { sql(sql).ok(); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-4437">[CALCITE-4437] + * The Sort rel should be decorrelated even though it has fetch or limit + * when it is not inside a Correlate</a>. + */ + @Test void testProjectSortLimitWithCorrelateInput() { + final String sql = "" + + "SELECT ename||deptno FROM\n" + + " (SELECT deptno, ename\n" + + " FROM\n" + + " (SELECT DISTINCT deptno FROM emp) t1,\n" + + " LATERAL (\n" + + " SELECT ename, sal\n" + + " FROM emp\n" + + " WHERE deptno = t1.deptno)\n" + + " ORDER BY ename DESC\n" + + " LIMIT 3)"; + sql(sql).ok(); + } + @Test void testSample() { final String sql = "select * from emp tablesample substitute('DATASET1') where empno > 5"; diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml index f9306eb..6eed6e8 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -4887,6 +4887,33 @@ LogicalSort(sort0=[$0], dir0=[ASC]) ]]> </Resource> </TestCase> + <TestCase name="testProjectSortLimitWithCorrelateInput"> + <Resource name="sql"> + <![CDATA[SELECT ename||deptno FROM + (SELECT deptno, ename + FROM + (SELECT DISTINCT deptno FROM emp) t1, + LATERAL ( + SELECT ename, sal + FROM emp + WHERE deptno = t1.deptno) + ORDER BY ename DESC + LIMIT 3)]]> + </Resource> + <Resource name="plan"> + <![CDATA[ +LogicalProject(EXPR$0=[||($1, CAST($0):VARCHAR NOT NULL)]) + LogicalSort(sort0=[$1], dir0=[DESC], fetch=[3]) + LogicalProject(DEPTNO=[$0], ENAME=[$1]) + LogicalJoin(condition=[=($0, $3)], joinType=[inner]) + LogicalAggregate(group=[{0}]) + LogicalProject(DEPTNO=[$7]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalProject(ENAME=[$1], SAL=[$5], DEPTNO=[$7]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> <TestCase name="testPushDownJoinCondition"> <Resource name="sql"> <![CDATA[select *