You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2019/01/31 18:52:22 UTC
[calcite] branch master updated: [CALCITE-2798] Remove ORDER BY in
sub-query, provided it has no LIMIT or OFFSET,
during SQL-to-RelNode conversion (Haisheng Yuan)
This is an automated email from the ASF dual-hosted git repository.
jhyde 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 cf363b5 [CALCITE-2798] Remove ORDER BY in sub-query, provided it has no LIMIT or OFFSET, during SQL-to-RelNode conversion (Haisheng Yuan)
cf363b5 is described below
commit cf363b5989d35bd8b4d5f14c7a99acea6e9f0f5a
Author: Haisheng Yuan <h....@alibaba-inc.com>
AuthorDate: Wed Jan 30 17:12:38 2019 -0800
[CALCITE-2798] Remove ORDER BY in sub-query, provided it has no LIMIT or OFFSET, during SQL-to-RelNode conversion (Haisheng Yuan)
Standard SQL says that ORDER BY in sub-queries should have no effect;
usually we prefer to make optimizations via planner rules, but we do
this change in SqlToRelConverter because a planner rule would have
difficulty telling whether a RelNode's consumers care about the order
in which it produces records.
Close apache/calcite#1021
---
.../apache/calcite/sql2rel/SqlToRelConverter.java | 20 +++++++-
.../apache/calcite/test/SqlToRelConverterTest.java | 21 ++++++++
.../java/org/apache/calcite/tools/PlannerTest.java | 59 ++++++++--------------
.../apache/calcite/test/SqlToRelConverterTest.xml | 49 ++++++++++++++++--
4 files changed, 105 insertions(+), 44 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index 2e58719..07561b2 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -773,6 +773,9 @@ public class SqlToRelConverter {
/**
* 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
@@ -789,9 +792,10 @@ public class SqlToRelConverter {
List<SqlNode> orderExprList,
SqlNode offset,
SqlNode fetch) {
- if (select.getOrderList() == null
+ if (!bb.top
+ || select.getOrderList() == null
|| select.getOrderList().getList().isEmpty()) {
- assert collation.getFieldCollations().isEmpty();
+ assert !bb.top || collation.getFieldCollations().isEmpty();
if ((offset == null
|| (offset instanceof SqlLiteral
&& ((SqlLiteral) offset).bigDecimalValue().equals(BigDecimal.ZERO)))
@@ -2944,6 +2948,18 @@ public class SqlToRelConverter {
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,
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 f13894c..5e43e1d 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -2968,6 +2968,27 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
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();
+ }
+
/**
* Visitor that checks that every {@link RelNode} in a tree is valid.
*
diff --git a/core/src/test/java/org/apache/calcite/tools/PlannerTest.java b/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
index 41b6684..bb706f7 100644
--- a/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
+++ b/core/src/test/java/org/apache/calcite/tools/PlannerTest.java
@@ -81,7 +81,6 @@ import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import org.hamcrest.Matcher;
-import org.junit.Ignore;
import org.junit.Test;
import java.util.ArrayList;
@@ -399,8 +398,8 @@ public class PlannerTest {
* <a href="https://issues.apache.org/jira/browse/CALCITE-2554">[CALCITE-2554]
* Enrich EnumerableJoin operator with order preserving information</a>.
*
- * Since left input to the join is sorted, and this join preserves order, there shouldn't be
- * any sort operator above the join.
+ * <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.
*/
@Test public void testRedundantSortOnJoinPlan() throws Exception {
RuleSet ruleSet =
@@ -437,7 +436,7 @@ public class PlannerTest {
/** Unit test that parses, validates, converts and
* plans for query using two duplicate order by.
- * The duplicate order by should be removed by SortRemoveRule. */
+ * The duplicate order by should be removed by SqlToRelConverter. */
@Test public void testDuplicateSortPlan() throws Exception {
runDuplicateSortCheck(
"select empid from ( "
@@ -445,28 +444,26 @@ public class PlannerTest {
+ "from emps "
+ "order by emps.deptno) "
+ "order by deptno",
- "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");
+ "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+ + " EnumerableProject(empid=[$0], deptno=[$1])\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 SortRemoveRule*/
+ * The duplicate order by should be removed by SqlToRelConverter. */
@Test public void testDuplicateSortPlanWithExpr() throws Exception {
runDuplicateSortCheck("select empid+deptno from ( "
+ "select empid, deptno "
+ "from emps "
+ "order by emps.deptno) "
+ "order by deptno",
- "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");
+ "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+ + " EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
+ + " EnumerableTableScan(table=[[hr, emps]])\n");
}
- @Test public void testTwoSortDontRemove() throws Exception {
+ @Test public void testTwoSortRemoveInnerSort() throws Exception {
runDuplicateSortCheck("select empid+deptno from ( "
+ "select empid, deptno "
+ "from emps "
@@ -474,16 +471,11 @@ public class PlannerTest {
+ "order by deptno",
"EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+ " EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
- + " EnumerableSort(sort0=[$0], dir0=[ASC])\n"
- + " EnumerableProject(empid=[$0], deptno=[$1])\n"
- + " EnumerableTableScan(table=[[hr, emps]])\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 ( "
@@ -492,14 +484,10 @@ public class PlannerTest {
+ " order by emps.deptno) "
+ ")"
+ "order by deptno",
- "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");
+ "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");
}
@Test public void testDuplicateSortPlanWithRemovedOver() throws Exception {
@@ -510,10 +498,9 @@ public class PlannerTest {
+ " order by emps.deptno) "
+ ")"
+ "order by deptno",
- "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");
+ "EnumerableSort(sort0=[$1], dir0=[ASC])\n"
+ + " EnumerableProject(EXPR$0=[+($0, $1)], deptno=[$1])\n"
+ + " EnumerableTableScan(table=[[hr, emps]])\n");
}
// If proper "SqlParseException, ValidationException, RelConversionException"
@@ -566,9 +553,7 @@ public class PlannerTest {
assertThat(toString(transform),
equalTo("EnumerableSort(sort0=[$1], dir0=[ASC])\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"));
+ + " EnumerableTableScan(table=[[hr, emps]])\n"));
}
/** Unit test that parses, validates, converts and plans. Planner is
@@ -1188,9 +1173,7 @@ public class PlannerTest {
assertThat(plan,
equalTo("LogicalSort(sort0=[$0], dir0=[ASC])\n"
+ " LogicalProject(psPartkey=[$0])\n"
- + " LogicalSort(sort0=[$0], sort1=[$1], dir0=[ASC], dir1=[ASC])\n"
- + " LogicalProject(psPartkey=[$0], psSupplyCost=[$1])\n"
- + " EnumerableTableScan(table=[[tpch, partsupp]])\n"));
+ + " EnumerableTableScan(table=[[tpch, partsupp]])\n"));
}
/** Test case for
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 d8539c7..7326357 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -2153,10 +2153,8 @@ LogicalProject(EMPNO=[$0], EXPR$1=[IS NOT TRUE($11)])
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalProject(ENAME=[$0])
- LogicalSort(sort0=[$1], dir0=[ASC])
- LogicalProject(ENAME=[$1], SAL=[$5])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+LogicalProject(ENAME=[$1])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
@@ -5475,4 +5473,47 @@ LogicalAggregate(group=[{0}], EXPR$1=[COLLECT($1) WITHIN GROUP ([2])], EXPR$2=[C
]]>
</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>
</Root>