This is an automated email from the ASF dual-hosted git repository. danny0405 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 734a7ac [CALCITE-4160] Add configuration(SqlToRelConverter.Config) to retain ORDER BY in sub-query (Jiatao Tao) 734a7ac is described below commit 734a7acd4cef2e3c2ef529844cf34e9fa8173aea Author: Jiatao Tao <245915...@qq.com> AuthorDate: Tue Sep 1 12:50:17 2020 +0800 [CALCITE-4160] Add configuration(SqlToRelConverter.Config) to retain ORDER BY in sub-query (Jiatao Tao) close apache/calcite#2127 --- .../apache/calcite/sql2rel/SqlToRelConverter.java | 41 +++++++++++++++++++--- .../apache/calcite/test/SqlToRelConverterTest.java | 6 ++++ .../apache/calcite/test/SqlToRelConverterTest.xml | 19 ++++++++++ 3 files changed, 61 insertions(+), 5 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 1b0a25c..3da06dd 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -824,10 +824,10 @@ public class SqlToRelConverter { List<SqlNode> orderExprList, SqlNode offset, SqlNode fetch) { - if (!bb.top + if (removeSortInSubquery(bb.top) || select.getOrderList() == null || select.getOrderList().getList().isEmpty()) { - assert !bb.top || collation.getFieldCollations().isEmpty(); + assert removeSortInSubquery(bb.top) || collation.getFieldCollations().isEmpty(); if ((offset == null || (offset instanceof SqlLiteral && ((SqlLiteral) offset).bigDecimalValue().equals(BigDecimal.ZERO))) @@ -866,6 +866,15 @@ public class SqlToRelConverter { } /** + * Returns whether we should remove the sort for the subsequent query conversion. + * + * @param topQuery Whether the query is in the top level. + */ + private boolean removeSortInSubquery(boolean topQuery) { + return config.isRemoveSortInSubquery() && !topQuery; + } + + /** * Returns whether a given node contains a {@link SqlInOperator}. * * @param node a RexNode tree @@ -3249,7 +3258,7 @@ public class SqlToRelConverter { return; } - if (!bb.top) { + if (removeSortInSubquery(bb.top)) { SqlNode offset = select.getOffset(); if ((offset == null || (offset instanceof SqlLiteral @@ -5865,6 +5874,15 @@ public class SqlToRelConverter { * the relational expressions. Default is * {@link HintStrategyTable#EMPTY}. */ HintStrategyTable getHintStrategyTable(); + + /** + * Returns whether to remove sort operator for a sub-query + * if the sort has no offset and fetch limit attributes. + * Because the remove does not change the semantics, in many cases, this is a promotion. + * + * <p> Default is true. + */ + boolean isRemoveSortInSubquery(); } /** Builder for a {@link Config}. */ @@ -5874,6 +5892,7 @@ public class SqlToRelConverter { private boolean createValuesRel = true; private boolean explain; private boolean expand = true; + private boolean removeSortInSubquery = true; private int inSubQueryThreshold = DEFAULT_IN_SUB_QUERY_THRESHOLD; private UnaryOperator<RelBuilder.Config> relBuilderConfigTransform = c -> c.withPushJoinCondition(true); @@ -5889,6 +5908,7 @@ public class SqlToRelConverter { this.createValuesRel = config.isCreateValuesRel(); this.explain = config.isExplain(); this.expand = config.isExpand(); + this.removeSortInSubquery = config.isRemoveSortInSubquery(); this.inSubQueryThreshold = config.getInSubQueryThreshold(); this.relBuilderConfigTransform = config.getRelBuilderConfigTransform(); this.relBuilderFactory = config.getRelBuilderFactory(); @@ -5921,6 +5941,11 @@ public class SqlToRelConverter { return this; } + public ConfigBuilder withRemoveSortInSubQuery(boolean removeSortInSubquery) { + this.removeSortInSubquery = removeSortInSubquery; + return this; + } + /** Whether to push down join conditions; default true. */ public ConfigBuilder withPushJoinCondition(boolean pushJoinCondition) { return withRelBuilderConfigTransform( @@ -5959,7 +5984,7 @@ public class SqlToRelConverter { /** Builds a {@link Config}. */ public Config build() { return new ConfigImpl(decorrelationEnabled, - trimUnusedFields, createValuesRel, explain, expand, + trimUnusedFields, createValuesRel, explain, expand, removeSortInSubquery, inSubQueryThreshold, relBuilderConfigTransform, relBuilderFactory, hintStrategyTable); } @@ -5973,6 +5998,7 @@ public class SqlToRelConverter { private final boolean createValuesRel; private final boolean explain; private final boolean expand; + private final boolean removeSortInSubquery; private final int inSubQueryThreshold; private final UnaryOperator<RelBuilder.Config> relBuilderConfigTransform; private final RelBuilderFactory relBuilderFactory; @@ -5980,7 +6006,7 @@ public class SqlToRelConverter { private ConfigImpl(boolean decorrelationEnabled, boolean trimUnusedFields, boolean createValuesRel, boolean explain, - boolean expand, int inSubQueryThreshold, + boolean expand, boolean removeSortInSubquery, int inSubQueryThreshold, UnaryOperator<RelBuilder.Config> relBuilderConfigTransform, RelBuilderFactory relBuilderFactory, HintStrategyTable hintStrategyTable) { @@ -5989,6 +6015,7 @@ public class SqlToRelConverter { this.createValuesRel = createValuesRel; this.explain = explain; this.expand = expand; + this.removeSortInSubquery = removeSortInSubquery; this.inSubQueryThreshold = inSubQueryThreshold; this.relBuilderConfigTransform = relBuilderConfigTransform; this.relBuilderFactory = relBuilderFactory; @@ -6034,6 +6061,10 @@ public class SqlToRelConverter { return expand; } + public boolean isRemoveSortInSubquery() { + return removeSortInSubquery; + } + public int getInSubQueryThreshold() { return inSubQueryThreshold; } 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 5b6aa21..b562eec 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -3936,6 +3936,12 @@ class SqlToRelConverterTest extends SqlToRelTestBase { sql(sql).ok(); } + @Test public void testSortInSubQuery() { + final String sql = "select * from (select empno from emp order by empno)"; + sql(sql).convertsTo("${planRemoveSort}"); + sql(sql).withConfig(c -> c.withRemoveSortInSubQuery(false)).convertsTo("${planKeepSort}"); + } + /** * Visitor that checks that every {@link RelNode} in a tree is valid. * 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 7fe6df1..a3db411 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -7038,4 +7038,23 @@ LogicalAggregate(group=[{0}], EXPR$1=[COUNT(DISTINCT $1)]) ]]> </Resource> </TestCase> + <TestCase name="testSortInSubQuery"> + <Resource name="sql"> + <![CDATA[select * from (select empno from emp order by empno)]]> + </Resource> + <Resource name="planRemoveSort"> + <![CDATA[ +LogicalProject(EMPNO=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="planKeepSort"> + <![CDATA[ +LogicalProject(EMPNO=[$0]) + LogicalSort(sort0=[$0], dir0=[ASC]) + LogicalProject(EMPNO=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> </Root>