[CALCITE-3121] VolcanoPlanner hangs due to removing ORDER BY from sub-query

core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java

@@ -775,9 +775,6 @@ private void distinctify(
   /**
    * Converts a query's ORDER BY clause, if any.
    *
-   * <p>Ignores the ORDER BY clause if the query is not top-level and FETCH or
-   * OFFSET are not present.
-   *
    * @param select        Query
    * @param bb            Blackboard
    * @param collation     Collation list
@@ -794,10 +791,9 @@ protected void convertOrder(
       List<SqlNode> orderExprList,
       SqlNode offset,
       SqlNode fetch) {
-    if (!bb.top
-        || select.getOrderList() == null
+    if (select.getOrderList() == null
         || select.getOrderList().getList().isEmpty()) {
-      assert !bb.top || collation.getFieldCollations().isEmpty();
+      assert collation.getFieldCollations().isEmpty();
       if ((offset == null
             || (offset instanceof SqlLiteral
                 && ((SqlLiteral) offset).bigDecimalValue().equals(BigDecimal.ZERO)))
@@ -3034,18 +3030,6 @@ protected void gatherOrderExprs(
     if (orderList == null) {
       return;
     }
-
-    if (!bb.top) {
-      SqlNode offset = select.getOffset();
-      if ((offset == null
-              || (offset instanceof SqlLiteral
-                  && ((SqlLiteral) offset).bigDecimalValue()
-                      .equals(BigDecimal.ZERO)))
-          && select.getFetch() == null) {
-        return;
-      }
-    }
-
     for (SqlNode orderItem : orderList) {
       collationList.add(
           convertOrderItem(select, orderItem, extraOrderExprs,

core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java

@@ -160,6 +160,7 @@
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Properties;
 import java.util.function.Predicate;
 
@@ -5580,6 +5581,54 @@ private Sql checkSubQuery(String sql) {
         .withPre(preProgram).with(program).check();
   }
 
+  @Test public void testSubQueryWithOrderByHang() {
+    String sql = "select n.n_regionkey from ( select * from "
+        + "( select * from sales.customer) t order by t.n_regionkey) n where n.n_nationkey >1 ";
+
+    VolcanoPlanner planner = new VolcanoPlanner(null, null);
+    planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
+
+    Tester dynamicTester = createDynamicTester().withDecorrelation(true)
+        .withClusterFactory(
+            relOptCluster -> RelOptCluster.create(planner, relOptCluster.getRexBuilder()));
+
+    RelRoot root = dynamicTester.convertSqlToRel(sql);
+
+    String planBefore = NL + RelOptUtil.toString(root.rel);
+    getDiffRepos().assertEquals("planBefore", "${planBefore}", planBefore);
+
+    PushProjector.ExprCondition exprCondition = expr -> {
+      if (expr instanceof RexCall) {
+        RexCall call = (RexCall) expr;
+        return "item".equals(call.getOperator().getName().toLowerCase(Locale.ROOT));
+      }
+      return false;
+    };
+    RuleSet ruleSet =
+        RuleSets.ofList(
+            FilterProjectTransposeRule.INSTANCE,
+            FilterMergeRule.INSTANCE,
+            ProjectMergeRule.INSTANCE,
+            new ProjectFilterTransposeRule(Project.class, Filter .class,
+                RelFactories.LOGICAL_BUILDER, exprCondition),
+            EnumerableRules.ENUMERABLE_PROJECT_RULE,
+            EnumerableRules.ENUMERABLE_FILTER_RULE,
+            EnumerableRules.ENUMERABLE_SORT_RULE,
+            EnumerableRules.ENUMERABLE_LIMIT_RULE,
+            EnumerableRules.ENUMERABLE_TABLE_SCAN_RULE);
+    Program program = Programs.of(ruleSet);
+
+    RelTraitSet toTraits =
+        root.rel.getCluster().traitSet()
+            .replace(0, EnumerableConvention.INSTANCE);
+
+    RelNode relAfter = program.run(planner, root.rel, toTraits,
+        Collections.emptyList(), Collections.emptyList());
+
+    String planAfter = NL + RelOptUtil.toString(relAfter);
+    getDiffRepos().assertEquals("planAfter", "${planAfter}", planAfter);
+  }
+
   /**
    * Custom implementation of {@link Filter} for use
    * in test case to verify that {@link FilterMultiJoinMergeRule}

core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java

@@ -3280,27 +3280,6 @@ private Tester getExtendedTester() {
     sql(sql).ok();
   }
 
-  @Test public void testOrderByRemoval1() {
-    final String sql = "select * from (\n"
-        + "  select empno from emp order by deptno offset 0) t\n"
-        + "order by empno desc";
-    sql(sql).ok();
-  }
-
-  @Test public void testOrderByRemoval2() {
-    final String sql = "select * from (\n"
-        + "  select empno from emp order by deptno offset 1) t\n"
-        + "order by empno desc";
-    sql(sql).ok();
-  }
-
-  @Test public void testOrderByRemoval3() {
-    final String sql = "select * from (\n"
-        + "  select empno from emp order by deptno limit 10) t\n"
-        + "order by empno";
-    sql(sql).ok();
-  }
-
   /**
    * Tests left join lateral with using
    */

core/src/test/java/org/apache/calcite/tools/PlannerTest.java

@@ -547,8 +547,8 @@ private void checkUnionPruning(String sql, String plan, RelOptRule... extraRules
    * <a href="https://issues.apache.org/jira/browse/CALCITE-2554">[CALCITE-2554]
    * Enrich EnumerableHashJoin operator with order preserving information</a>.
    *
-   * <p>Since the left input to the join is sorted, and this join preserves
-   * order, there shouldn't be any sort operator above the join.
+   * Since left input to the join is sorted, and this join preserves order, there shouldn't be
+   * any sort operator above the join.
    */
   @Test public void testRedundantSortOnJoinPlan() throws Exception {
     RuleSet ruleSet =
@@ -585,46 +585,53 @@ private void checkUnionPruning(String sql, String plan, RelOptRule... extraRules
 
   /** Unit test that parses, validates, converts and
    * plans for query using two duplicate order by.
-   * The duplicate order by should be removed by SqlToRelConverter. */
+   * The duplicate order by should be removed by SortRemoveRule. */
   @Test public void testDuplicateSortPlan() throws Exception {
     runDuplicateSortCheck(
         "select empid from ( "
         + "select * "
         + "from emps "
         + "order by emps.deptno) "
         + "order by deptno",
-        "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
-        + "  EnumerableProject(empid=[$0], deptno=[$1])\n"
-        + "    EnumerableTableScan(table=[[hr, emps]])\n");
+        "EnumerableProject(empid=[$0], deptno=[$1])\n"
+        + "  EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+        + "    EnumerableProject(empid=[$0], deptno=[$1], name=[$2], salary=[$3], commission=[$4])\n"
+        + "      EnumerableTableScan(table=[[hr, emps]])\n");
   }
 
   /** Unit test that parses, validates, converts and
    * plans for query using two duplicate order by.
-   * The duplicate order by should be removed by SqlToRelConverter. */
+   * The duplicate order by should be removed by SortRemoveRule*/
   @Test public void testDuplicateSortPlanWithExpr() throws Exception {
     runDuplicateSortCheck("select empid+deptno from ( "
         + "select empid, deptno "
         + "from emps "
         + "order by emps.deptno) "
         + "order by deptno",
-        "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
-        + "  EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
-        + "    EnumerableTableScan(table=[[hr, emps]])\n");
+        "EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
+        + "  EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+        + "    EnumerableProject(empid=[$0], deptno=[$1])\n"
+        + "      EnumerableTableScan(table=[[hr, emps]])\n");
   }
 
-  @Test public void testTwoSortRemoveInnerSort() throws Exception {
+  @Test public void testTwoSortDontRemove() throws Exception {
     runDuplicateSortCheck("select empid+deptno from ( "
         + "select empid, deptno "
         + "from emps "
         + "order by empid) "
         + "order by deptno",
         "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
         + "  EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
-        + "    EnumerableTableScan(table=[[hr, emps]])\n");
+        + "    EnumerableSort(sort0=[$0], dir0=[ASC])\n"
+        + "      EnumerableProject(empid=[$0], deptno=[$1])\n"
+        + "        EnumerableTableScan(table=[[hr, emps]])\n");
   }
 
   /** Tests that outer order by is not removed since window function
    * might reorder the rows in-between */
+  @Ignore("Node [rel#22:Subset#3.ENUMERABLE.[2]] could not be implemented; planner state:\n"
+      + "\n"
+      + "Root: rel#22:Subset#3.ENUMERABLE.[2]")
   @Test public void testDuplicateSortPlanWithOver() throws Exception {
     runDuplicateSortCheck("select emp_cnt, empid+deptno from ( "
         + "select empid, deptno, count(*) over (partition by deptno) emp_cnt from ( "
@@ -633,10 +640,14 @@ private void checkUnionPruning(String sql, String plan, RelOptRule... extraRules
         + "   order by emps.deptno) "
         + ")"
         + "order by deptno",
-        "EnumerableSort(sort0=[$2], dir0=[ASC])\n"
-        + "  EnumerableProject(emp_cnt=[$5], EXPR$1=[+($0, $1)], deptno=[$1])\n"
-        + "    EnumerableWindow(window#0=[window(partition {1} order by [] range between UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING aggs [COUNT()])])\n"
-        + "      EnumerableTableScan(table=[[hr, emps]])\n");
+        "EnumerableProject(EXPR$0=[$0])\n"
+        + "  EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+        + "    EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
+        + "      EnumerableProject(empid=[$0], deptno=[$1], $2=[$2])\n"
+        + "        EnumerableWindow(window#0=[window(partition {1} order by [] range between UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING aggs [COUNT()])])\n"
+        + "          EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+        + "            EnumerableProject(empid=[$0], deptno=[$1])\n"
+        + "              EnumerableTableScan(table=[[hr, emps]])\n");
   }
 
   @Test public void testDuplicateSortPlanWithRemovedOver() throws Exception {
@@ -647,9 +658,10 @@ private void checkUnionPruning(String sql, String plan, RelOptRule... extraRules
         + "   order by emps.deptno) "
         + ")"
         + "order by deptno",
-        "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
-        + "  EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
-        + "    EnumerableTableScan(table=[[hr, emps]])\n");
+        "EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
+        + "  EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+        + "    EnumerableProject(empid=[$0], deptno=[$1])\n"
+        + "      EnumerableTableScan(table=[[hr, emps]])\n");
   }
 
   // If proper "SqlParseException, ValidationException, RelConversionException"
@@ -702,7 +714,9 @@ private void runDuplicateSortCheck(String sql, String plan) throws Exception {
     assertThat(toString(transform),
         equalTo("EnumerableSort(sort0=[$1], dir0=[ASC])\n"
             + "  EnumerableProject(empid=[$0], deptno=[$1])\n"
-            + "    EnumerableTableScan(table=[[hr, emps]])\n"));
+            + "    EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+            + "      EnumerableProject(empid=[$0], deptno=[$1], name=[$2], salary=[$3], commission=[$4])\n"
+            + "        EnumerableTableScan(table=[[hr, emps]])\n"));
   }
 
   /** Test case for
@@ -1339,7 +1353,9 @@ public RelDataType deriveType(SqlValidator validator,
     assertThat(plan,
         equalTo("LogicalSort(sort0=[$0], dir0=[ASC])\n"
         + "  LogicalProject(psPartkey=[$0])\n"
-        + "    EnumerableTableScan(table=[[tpch, partsupp]])\n"));
+        + "    LogicalSort(sort0=[$0], sort1=[$1], dir0=[ASC], dir1=[ASC])\n"
+        + "      LogicalProject(psPartkey=[$0], psSupplyCost=[$1])\n"
+        + "        EnumerableTableScan(table=[[tpch, partsupp]])\n"));
   }
 
   /** Test case for

core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml

@@ -10994,6 +10994,30 @@ MyProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], C
 MultiJoin(joinFilter=[true], isFullOuterJoin=[false], joinTypes=[[INNER, LEFT]], outerJoinConditions=[[NULL, =($7, $9)]], projFields=[[{0, 1, 2, 3, 4, 5, 6, 7, 8}, {0, 1}]], postJoinFilter=[>($9, 3)])
   LogicalTableScan(table=[[CATALOG, SALES, EMP]])
   LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testSubQueryWithOrderByHang">
+        <Resource name="sql">
+            <![CDATA[select n.n_regionkey from ( select * from ( select * from sales.customer) t order by t.n_regionkey) n where n.n_nationkey >1]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(N_REGIONKEY=[ITEM($0, 'N_REGIONKEY')])
+  LogicalFilter(condition=[>(ITEM($0, 'N_NATIONKEY'), 1)])
+    LogicalProject(**=[$0])
+      LogicalSort(sort0=[$1], dir0=[ASC])
+        LogicalProject(**=[$0], EXPR$1=[ITEM($0, 'N_REGIONKEY')])
+          LogicalTableScan(table=[[CATALOG, SALES, CUSTOMER]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+EnumerableProject(N_REGIONKEY=[ITEM($0, 'N_REGIONKEY')])
+  EnumerableFilter(condition=[>(ITEM($0, 'N_NATIONKEY'), 1)])
+    EnumerableSort(sort0=[$1], dir0=[ASC])
+      EnumerableProject(**=[$0], EXPR$1=[ITEM($0, 'N_REGIONKEY')])
+        EnumerableTableScan(table=[[CATALOG, SALES, CUSTOMER]])
 ]]>
         </Resource>
     </TestCase>

core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml

@@ -2338,8 +2338,10 @@ LogicalProject(EMPNO=[$0], EXPR$1=[IS NOT TRUE($11)])
         </Resource>
         <Resource name="plan">
             <![CDATA[
-LogicalProject(ENAME=[$1])
-  LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+LogicalProject(ENAME=[$0])
+  LogicalSort(sort0=[$1], dir0=[ASC])
+    LogicalProject(ENAME=[$1], SAL=[$5])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>
     </TestCase>
@@ -5878,52 +5880,6 @@ group by deptno]]>
 LogicalAggregate(group=[{0}], EXPR$1=[COLLECT($1) WITHIN GROUP ([2])], EXPR$2=[COUNT()])
   LogicalProject(DEPTNO=[$7], EMPNO=[$0], $f2=[AND(<>($0, 1), <>($0, 2))])
     LogicalTableScan(table=[[CATALOG, SALES, EMP]])
-]]>
-        </Resource>
-    </TestCase>
-    <TestCase name="testOrderByRemoval1">
-        <Resource name="sql">
-            <![CDATA[select * from (
-  select empno from emp order by deptno offset 0) t
-order by empno desc]]>
-        </Resource>
-        <Resource name="plan">
-            <![CDATA[
-LogicalSort(sort0=[$0], dir0=[DESC])
-  LogicalProject(EMPNO=[$0])
-    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
-]]>
-        </Resource>
-    </TestCase>
-    <TestCase name="testOrderByRemoval2">
-        <Resource name="sql">
-            <![CDATA[select * from (
-  select empno from emp order by deptno offset 1) t
-order by empno desc]]>
-        </Resource>
-        <Resource name="plan">
-            <![CDATA[
-LogicalSort(sort0=[$0], dir0=[DESC])
-  LogicalProject(EMPNO=[$0])
-    LogicalSort(sort0=[$1], dir0=[ASC], offset=[1])
-      LogicalProject(EMPNO=[$0], DEPTNO=[$7])
-        LogicalTableScan(table=[[CATALOG, SALES, EMP]])
-]]>
-        </Resource>
-    </TestCase>
-    <TestCase name="testOrderByRemoval3">
-        <Resource name="sql">
-            <![CDATA[select * from (
-  select empno from emp order by deptno limit 10) t
-order by empno]]>
-        </Resource>
-        <Resource name="plan">
-            <![CDATA[
-LogicalSort(sort0=[$0], dir0=[ASC])
-  LogicalProject(EMPNO=[$0])
-    LogicalSort(sort0=[$1], dir0=[ASC], fetch=[10])
-      LogicalProject(EMPNO=[$0], DEPTNO=[$7])
-        LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>
     </TestCase>