From 2fec4bd441ebf40cedead5dcbda4492addd6d7d6 Mon Sep 17 00:00:00 2001 From: kasiafi <30203062+kasiafi@users.noreply.github.com> Date: Sun, 26 May 2019 19:53:42 +0200 Subject: [PATCH] Simplify OFFSET implementation This change is based on RowNumberNode's property of keeping order of its input. In OFFSET implementation, a RowNumberNode and a FilterNode are used for rows selection. Before this change, if there was ORDER BY in the query, it was handled by one of the specialised rules which ensured that sorting was replicated over RowNumberNode. Since RowNumberNode respects sorted order of its input, it is unnecessary to add a SortNode. --- .../prestosql/sql/planner/PlanOptimizers.java | 24 +--- ...setOverOther.java => ImplementOffset.java} | 13 +-- .../rule/ImplementOffsetOverProjectSort.java | 109 ----------------- .../rule/ImplementOffsetOverProjectTopN.java | 110 ------------------ .../rule/ImplementOffsetOverSort.java | 107 ----------------- .../rule/ImplementOffsetOverTopN.java | 108 ----------------- .../sql/planner/TestLogicalPlanner.java | 58 ++++----- ...verOther.java => TestImplementOffset.java} | 38 +++++- .../TestImplementOffsetOverProjectSort.java | 66 ----------- .../TestImplementOffsetOverProjectTopN.java | 69 ----------- .../rule/TestImplementOffsetOverSort.java | 63 ---------- .../rule/TestImplementOffsetOverTopN.java | 66 ----------- 12 files changed, 68 insertions(+), 763 deletions(-) rename presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/{ImplementOffsetOverOther.java => ImplementOffset.java} (87%) delete mode 100644 presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectSort.java delete mode 100644 presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectTopN.java delete mode 100644 presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverSort.java delete mode 100644 presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverTopN.java rename presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/{TestImplementOffsetOverOther.java => TestImplementOffset.java} (56%) delete mode 100644 presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectSort.java delete mode 100644 presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectTopN.java delete mode 100644 presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverSort.java delete mode 100644 presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverTopN.java diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/PlanOptimizers.java b/presto-main/src/main/java/io/prestosql/sql/planner/PlanOptimizers.java index bef5efdd9ec9..0a4fbc2ef7d8 100644 --- a/presto-main/src/main/java/io/prestosql/sql/planner/PlanOptimizers.java +++ b/presto-main/src/main/java/io/prestosql/sql/planner/PlanOptimizers.java @@ -46,11 +46,7 @@ import io.prestosql.sql.planner.iterative.rule.GatherAndMergeWindows; import io.prestosql.sql.planner.iterative.rule.ImplementBernoulliSampleAsFilter; import io.prestosql.sql.planner.iterative.rule.ImplementFilteredAggregations; -import io.prestosql.sql.planner.iterative.rule.ImplementOffsetOverOther; -import io.prestosql.sql.planner.iterative.rule.ImplementOffsetOverProjectSort; -import io.prestosql.sql.planner.iterative.rule.ImplementOffsetOverProjectTopN; -import io.prestosql.sql.planner.iterative.rule.ImplementOffsetOverSort; -import io.prestosql.sql.planner.iterative.rule.ImplementOffsetOverTopN; +import io.prestosql.sql.planner.iterative.rule.ImplementOffset; import io.prestosql.sql.planner.iterative.rule.InlineProjections; import io.prestosql.sql.planner.iterative.rule.MergeFilters; import io.prestosql.sql.planner.iterative.rule.MergeLimitOverProjectWithSort; @@ -343,23 +339,9 @@ public PlanOptimizers( estimatedExchangesCostCalculator, // Temporary hack: separate optimizer step to avoid the sample node being replaced by filter before pushing // it to table scan node - ImmutableSet.of(new ImplementBernoulliSampleAsFilter())), - new IterativeOptimizer( - ruleStats, - statsCalculator, - estimatedExchangesCostCalculator, ImmutableSet.of( - new ImplementOffsetOverTopN(), - new ImplementOffsetOverProjectTopN(), - new ImplementOffsetOverSort(), - new ImplementOffsetOverProjectSort())), - new IterativeOptimizer( - ruleStats, - statsCalculator, - estimatedExchangesCostCalculator, - // Temporary hack: separate optimizer step to avoid a plan that's missing a SortNode over a RowNumber node - // if the rules fires too early - ImmutableSet.of(new ImplementOffsetOverOther())), + new ImplementBernoulliSampleAsFilter(), + new ImplementOffset())), simplifyOptimizer, new UnaliasSymbolReferences(), new IterativeOptimizer( diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverOther.java b/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffset.java similarity index 87% rename from presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverOther.java rename to presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffset.java index 87c2608104e8..9e0a78b1389a 100644 --- a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverOther.java +++ b/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffset.java @@ -23,8 +23,6 @@ import io.prestosql.sql.planner.plan.OffsetNode; import io.prestosql.sql.planner.plan.ProjectNode; import io.prestosql.sql.planner.plan.RowNumberNode; -import io.prestosql.sql.planner.plan.SortNode; -import io.prestosql.sql.planner.plan.TopNNode; import io.prestosql.sql.tree.ComparisonExpression; import io.prestosql.sql.tree.GenericLiteral; @@ -32,13 +30,12 @@ import static io.prestosql.spi.type.BigintType.BIGINT; import static io.prestosql.sql.planner.plan.Patterns.offset; -import static io.prestosql.sql.planner.plan.Patterns.source; /** * Transforms: * 
  * - Offset (row count = x)
- *    - Source (other than Sort, TopN)
+ *    - Source
  *
* Into: * 
@@ -47,12 +44,14 @@
  *       - RowNumber
  *          - Source
  *
+ * Relies on RowNumberNode's property of keeping order of its input. + * If the query contains an ORDER BY clause, the sorted order + * will be respected when leading rows are removed. */ -public class ImplementOffsetOverOther +public class ImplementOffset implements Rule { - private static final Pattern PATTERN = offset() - .with(source().matching(node -> !(node instanceof TopNNode || node instanceof SortNode))); + private static final Pattern PATTERN = offset(); @Override public Pattern getPattern() diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectSort.java b/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectSort.java deleted file mode 100644 index 2fcc7162774c..000000000000 --- a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectSort.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.matching.Capture; -import io.prestosql.matching.Captures; -import io.prestosql.matching.Pattern; -import io.prestosql.spi.block.SortOrder; -import io.prestosql.sql.planner.OrderingScheme; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.iterative.Rule; -import io.prestosql.sql.planner.plan.FilterNode; -import io.prestosql.sql.planner.plan.OffsetNode; -import io.prestosql.sql.planner.plan.ProjectNode; -import io.prestosql.sql.planner.plan.RowNumberNode; -import io.prestosql.sql.planner.plan.SortNode; -import io.prestosql.sql.tree.ComparisonExpression; -import io.prestosql.sql.tree.GenericLiteral; - -import java.util.Optional; - -import static io.prestosql.matching.Capture.newCapture; -import static io.prestosql.spi.type.BigintType.BIGINT; -import static io.prestosql.sql.planner.plan.Patterns.offset; -import static io.prestosql.sql.planner.plan.Patterns.project; -import static io.prestosql.sql.planner.plan.Patterns.sort; -import static io.prestosql.sql.planner.plan.Patterns.source; - -/** - * Transforms: - * 
- * - Offset (row count = x)
- *    - Project (prunes sort symbols no longer useful)
- *       - Sort (order by a, b, c)
- *
- * Into: - * 
- * - Project (prunes rowNumber symbol and sort symbols no longer useful)
- *    - Sort (order by rowNumber)
- *       - Filter (rowNumber > x)
- *          - RowNumber
- *             - Sort (order by a, b, c)
- *
- */ -public class ImplementOffsetOverProjectSort - implements Rule -{ - private static final Capture PROJECT = newCapture(); - private static final Capture SORT = newCapture(); - - private static final Pattern PATTERN = offset() - .with(source().matching( - project().capturedAs(PROJECT).matching(ProjectNode::isIdentity) - .with(source().matching( - sort().capturedAs(SORT))))); - - @Override - public Pattern getPattern() - { - return PATTERN; - } - - @Override - public Result apply(OffsetNode parent, Captures captures, Context context) - { - ProjectNode project = captures.get(PROJECT); - SortNode sort = captures.get(SORT); - - Symbol rowNumberSymbol = context.getSymbolAllocator().newSymbol("row_number", BIGINT); - - RowNumberNode rowNumberNode = new RowNumberNode( - context.getIdAllocator().getNextId(), - sort, - ImmutableList.of(), - rowNumberSymbol, - Optional.empty(), - Optional.empty()); - - FilterNode filterNode = new FilterNode( - context.getIdAllocator().getNextId(), - rowNumberNode, - new ComparisonExpression( - ComparisonExpression.Operator.GREATER_THAN, - rowNumberSymbol.toSymbolReference(), - new GenericLiteral("BIGINT", Long.toString(parent.getCount())))); - - SortNode sortNode = new SortNode( - context.getIdAllocator().getNextId(), - filterNode, - new OrderingScheme(ImmutableList.of(rowNumberSymbol), ImmutableMap.of(rowNumberSymbol, SortOrder.ASC_NULLS_FIRST))); - - ProjectNode projectNode = (ProjectNode) project.replaceChildren(ImmutableList.of(sortNode)); - - return Result.ofPlanNode(projectNode); - } -} diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectTopN.java b/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectTopN.java deleted file mode 100644 index 7de6abe9515a..000000000000 --- a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverProjectTopN.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.matching.Capture; -import io.prestosql.matching.Captures; -import io.prestosql.matching.Pattern; -import io.prestosql.spi.block.SortOrder; -import io.prestosql.sql.planner.OrderingScheme; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.iterative.Rule; -import io.prestosql.sql.planner.plan.FilterNode; -import io.prestosql.sql.planner.plan.OffsetNode; -import io.prestosql.sql.planner.plan.ProjectNode; -import io.prestosql.sql.planner.plan.RowNumberNode; -import io.prestosql.sql.planner.plan.SortNode; -import io.prestosql.sql.planner.plan.TopNNode; -import io.prestosql.sql.tree.ComparisonExpression; -import io.prestosql.sql.tree.GenericLiteral; - -import java.util.Optional; - -import static io.prestosql.matching.Capture.newCapture; -import static io.prestosql.spi.type.BigintType.BIGINT; -import static io.prestosql.sql.planner.plan.Patterns.offset; -import static io.prestosql.sql.planner.plan.Patterns.project; -import static io.prestosql.sql.planner.plan.Patterns.source; -import static io.prestosql.sql.planner.plan.Patterns.topN; - -/** - * Transforms: - * 
- * - Offset (row count = x)
- *    - Project (prunes sort symbols no longer useful)
- *       - TopN (order by a, b, c)
- *
- * Into: - * 
- * - Project (prunes rowNumber symbol and sort symbols no longer useful)
- *    - Sort (order by rowNumber)
- *       - Filter (rowNumber > x)
- *          - RowNumber
- *             - TopN (order by a, b, c)
- *
- */ -public class ImplementOffsetOverProjectTopN - implements Rule -{ - private static final Capture PROJECT = newCapture(); - private static final Capture TOPN = newCapture(); - - private static final Pattern PATTERN = offset() - .with(source().matching( - project().capturedAs(PROJECT).matching(ProjectNode::isIdentity) - .with(source().matching( - topN().capturedAs(TOPN))))); - - @Override - public Pattern getPattern() - { - return PATTERN; - } - - @Override - public Result apply(OffsetNode parent, Captures captures, Context context) - { - ProjectNode project = captures.get(PROJECT); - TopNNode topN = captures.get(TOPN); - - Symbol rowNumberSymbol = context.getSymbolAllocator().newSymbol("row_number", BIGINT); - - RowNumberNode rowNumberNode = new RowNumberNode( - context.getIdAllocator().getNextId(), - topN, - ImmutableList.of(), - rowNumberSymbol, - Optional.empty(), - Optional.empty()); - - FilterNode filterNode = new FilterNode( - context.getIdAllocator().getNextId(), - rowNumberNode, - new ComparisonExpression( - ComparisonExpression.Operator.GREATER_THAN, - rowNumberSymbol.toSymbolReference(), - new GenericLiteral("BIGINT", Long.toString(parent.getCount())))); - - SortNode sortNode = new SortNode( - context.getIdAllocator().getNextId(), - filterNode, - new OrderingScheme(ImmutableList.of(rowNumberSymbol), ImmutableMap.of(rowNumberSymbol, SortOrder.ASC_NULLS_FIRST))); - - ProjectNode projectNode = (ProjectNode) project.replaceChildren(ImmutableList.of(sortNode)); - - return Result.ofPlanNode(projectNode); - } -} diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverSort.java b/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverSort.java deleted file mode 100644 index cf5b695f9b21..000000000000 --- a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverSort.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.matching.Capture; -import io.prestosql.matching.Captures; -import io.prestosql.matching.Pattern; -import io.prestosql.spi.block.SortOrder; -import io.prestosql.sql.planner.OrderingScheme; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.iterative.Rule; -import io.prestosql.sql.planner.plan.Assignments; -import io.prestosql.sql.planner.plan.FilterNode; -import io.prestosql.sql.planner.plan.OffsetNode; -import io.prestosql.sql.planner.plan.ProjectNode; -import io.prestosql.sql.planner.plan.RowNumberNode; -import io.prestosql.sql.planner.plan.SortNode; -import io.prestosql.sql.tree.ComparisonExpression; -import io.prestosql.sql.tree.GenericLiteral; - -import java.util.Optional; - -import static io.prestosql.matching.Capture.newCapture; -import static io.prestosql.spi.type.BigintType.BIGINT; -import static io.prestosql.sql.planner.plan.Patterns.offset; -import static io.prestosql.sql.planner.plan.Patterns.sort; -import static io.prestosql.sql.planner.plan.Patterns.source; - -/** - * Transforms: - * 
- * - Offset (row count = x)
- *    - Sort (order by a, b, c)
- *
- * Into: - * 
- * - Project (prune rowNumber symbol)
- *    - Sort (order by rowNumber)
- *       - Filter (rowNumber > x)
- *          - RowNumber
- *             - Sort (order by a, b, c)
- *
- */ -public class ImplementOffsetOverSort - implements Rule -{ - private static final Capture SORT = newCapture(); - - private static final Pattern PATTERN = offset() - .with(source().matching( - sort().capturedAs(SORT))); - - @Override - public Pattern getPattern() - { - return PATTERN; - } - - @Override - public Result apply(OffsetNode parent, Captures captures, Context context) - { - SortNode sort = captures.get(SORT); - - Symbol rowNumberSymbol = context.getSymbolAllocator().newSymbol("row_number", BIGINT); - - RowNumberNode rowNumberNode = new RowNumberNode( - context.getIdAllocator().getNextId(), - sort, - ImmutableList.of(), - rowNumberSymbol, - Optional.empty(), - Optional.empty()); - - FilterNode filterNode = new FilterNode( - context.getIdAllocator().getNextId(), - rowNumberNode, - new ComparisonExpression( - ComparisonExpression.Operator.GREATER_THAN, - rowNumberSymbol.toSymbolReference(), - new GenericLiteral("BIGINT", Long.toString(parent.getCount())))); - - SortNode sortNode = new SortNode( - context.getIdAllocator().getNextId(), - filterNode, - new OrderingScheme(ImmutableList.of(rowNumberSymbol), ImmutableMap.of(rowNumberSymbol, SortOrder.ASC_NULLS_FIRST))); - - ProjectNode projectNode = new ProjectNode( - context.getIdAllocator().getNextId(), - sortNode, - Assignments.identity(sort.getOutputSymbols())); - - return Result.ofPlanNode(projectNode); - } -} diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverTopN.java b/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverTopN.java deleted file mode 100644 index 03f0f8b3c6f7..000000000000 --- a/presto-main/src/main/java/io/prestosql/sql/planner/iterative/rule/ImplementOffsetOverTopN.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.matching.Capture; -import io.prestosql.matching.Captures; -import io.prestosql.matching.Pattern; -import io.prestosql.spi.block.SortOrder; -import io.prestosql.sql.planner.OrderingScheme; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.iterative.Rule; -import io.prestosql.sql.planner.plan.Assignments; -import io.prestosql.sql.planner.plan.FilterNode; -import io.prestosql.sql.planner.plan.OffsetNode; -import io.prestosql.sql.planner.plan.ProjectNode; -import io.prestosql.sql.planner.plan.RowNumberNode; -import io.prestosql.sql.planner.plan.SortNode; -import io.prestosql.sql.planner.plan.TopNNode; -import io.prestosql.sql.tree.ComparisonExpression; -import io.prestosql.sql.tree.GenericLiteral; - -import java.util.Optional; - -import static io.prestosql.matching.Capture.newCapture; -import static io.prestosql.spi.type.BigintType.BIGINT; -import static io.prestosql.sql.planner.plan.Patterns.offset; -import static io.prestosql.sql.planner.plan.Patterns.source; -import static io.prestosql.sql.planner.plan.Patterns.topN; - -/** - * Transforms: - * 
- * - Offset (row count = x)
- *    - TopN (order by a, b, c)
- *
- * Into: - * 
- * - Project (prune rowNumber symbol)
- *    - Sort (order by rowNumber)
- *       - Filter (rowNumber > x)
- *          - RowNumber
- *             - TopN (order by a, b, c)
- *
- */ -public class ImplementOffsetOverTopN - implements Rule -{ - private static final Capture TOPN = newCapture(); - - private static final Pattern PATTERN = offset() - .with(source().matching( - topN().capturedAs(TOPN))); - - @Override - public Pattern getPattern() - { - return PATTERN; - } - - @Override - public Result apply(OffsetNode parent, Captures captures, Context context) - { - TopNNode topN = captures.get(TOPN); - - Symbol rowNumberSymbol = context.getSymbolAllocator().newSymbol("row_number", BIGINT); - - RowNumberNode rowNumberNode = new RowNumberNode( - context.getIdAllocator().getNextId(), - topN, - ImmutableList.of(), - rowNumberSymbol, - Optional.empty(), - Optional.empty()); - - FilterNode filterNode = new FilterNode( - context.getIdAllocator().getNextId(), - rowNumberNode, - new ComparisonExpression( - ComparisonExpression.Operator.GREATER_THAN, - rowNumberSymbol.toSymbolReference(), - new GenericLiteral("BIGINT", Long.toString(parent.getCount())))); - - SortNode sortNode = new SortNode( - context.getIdAllocator().getNextId(), - filterNode, - new OrderingScheme(ImmutableList.of(rowNumberSymbol), ImmutableMap.of(rowNumberSymbol, SortOrder.ASC_NULLS_FIRST))); - - ProjectNode projectNode = new ProjectNode( - context.getIdAllocator().getNextId(), - sortNode, - Assignments.identity(topN.getOutputSymbols())); - - return Result.ofPlanNode(projectNode); - } -} diff --git a/presto-main/src/test/java/io/prestosql/sql/planner/TestLogicalPlanner.java b/presto-main/src/test/java/io/prestosql/sql/planner/TestLogicalPlanner.java index 9e6caaff836e..fdd207fcb074 100644 --- a/presto-main/src/test/java/io/prestosql/sql/planner/TestLogicalPlanner.java +++ b/presto-main/src/test/java/io/prestosql/sql/planner/TestLogicalPlanner.java @@ -102,7 +102,6 @@ import static io.prestosql.sql.planner.plan.JoinNode.DistributionType.REPLICATED; import static io.prestosql.sql.planner.plan.JoinNode.Type.INNER; import static io.prestosql.sql.planner.plan.JoinNode.Type.LEFT; -import static io.prestosql.sql.tree.SortItem.NullOrdering.FIRST; import static io.prestosql.sql.tree.SortItem.NullOrdering.LAST; import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING; import static io.prestosql.sql.tree.SortItem.Ordering.DESCENDING; @@ -879,7 +878,7 @@ public void testOffset() strictProject( ImmutableMap.of("name", new ExpressionMatcher("name")), filter( - "(row_num > BIGINT '2')", + "row_num > BIGINT '2'", rowNumber( pattern -> pattern .partitionBy(ImmutableList.of()), @@ -892,43 +891,36 @@ public void testOffset() any( strictProject( ImmutableMap.of("name", new ExpressionMatcher("name")), - any( - sort( - ImmutableList.of(sort("row_num", ASCENDING, FIRST)), - any( - filter( - "(row_num > BIGINT '2')", - rowNumber( - pattern -> pattern - .partitionBy(ImmutableList.of()), - any( - sort( - ImmutableList.of(sort("regionkey", ASCENDING, LAST)), - any( - tableScan("nation", ImmutableMap.of("NAME", "name", "REGIONKEY", "regionkey")))))) - .withAlias("row_num", new RowNumberSymbolMatcher())))))))); + filter( + "row_num > BIGINT '2'", + rowNumber( + pattern -> pattern + .partitionBy(ImmutableList.of()), + anyTree( + sort( + ImmutableList.of(sort("regionkey", ASCENDING, LAST)), + any( + tableScan("nation", ImmutableMap.of("NAME", "name", "REGIONKEY", "regionkey")))))) + .withAlias("row_num", new RowNumberSymbolMatcher()))))); assertPlan( "SELECT name FROM nation ORDER BY regionkey OFFSET 2 ROWS FETCH NEXT 5 ROWS ONLY", any( strictProject( ImmutableMap.of("name", new ExpressionMatcher("name")), - any( - sort( - ImmutableList.of(sort("row_num", ASCENDING, FIRST)), + filter( + "row_num > BIGINT '2'", + rowNumber( + pattern -> pattern + .partitionBy(ImmutableList.of()), any( - filter( - "(row_num > BIGINT '2')", - rowNumber( - pattern -> pattern - .partitionBy(ImmutableList.of()), - topN( - 7, - ImmutableList.of(sort("regionkey", ASCENDING, LAST)), - TopNNode.Step.FINAL, - anyTree( - tableScan("nation", ImmutableMap.of("NAME", "name", "REGIONKEY", "regionkey"))))) - .withAlias("row_num", new RowNumberSymbolMatcher())))))))); + topN( + 7, + ImmutableList.of(sort("regionkey", ASCENDING, LAST)), + TopNNode.Step.FINAL, + anyTree( + tableScan("nation", ImmutableMap.of("NAME", "name", "REGIONKEY", "regionkey")))))) + .withAlias("row_num", new RowNumberSymbolMatcher()))))); assertPlan( "SELECT name FROM nation OFFSET 2 ROWS FETCH NEXT 5 ROWS ONLY", @@ -936,7 +928,7 @@ public void testOffset() strictProject( ImmutableMap.of("name", new ExpressionMatcher("name")), filter( - "(row_num > BIGINT '2')", + "row_num > BIGINT '2'", rowNumber( pattern -> pattern .partitionBy(ImmutableList.of()), diff --git a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverOther.java b/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffset.java similarity index 56% rename from presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverOther.java rename to presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffset.java index 27c1049a5da3..741eada368d5 100644 --- a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverOther.java +++ b/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffset.java @@ -23,16 +23,19 @@ import static io.prestosql.sql.planner.assertions.PlanMatchPattern.filter; import static io.prestosql.sql.planner.assertions.PlanMatchPattern.rowNumber; +import static io.prestosql.sql.planner.assertions.PlanMatchPattern.sort; import static io.prestosql.sql.planner.assertions.PlanMatchPattern.strictProject; import static io.prestosql.sql.planner.assertions.PlanMatchPattern.values; +import static io.prestosql.sql.tree.SortItem.NullOrdering.FIRST; +import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING; -public class TestImplementOffsetOverOther +public class TestImplementOffset extends BaseRuleTest { @Test - public void testReplaceOffsetOverOther() + public void testReplaceOffsetOverValues() { - tester().assertThat(new ImplementOffsetOverOther()) + tester().assertThat(new ImplementOffset()) .on(p -> { Symbol a = p.symbol("a"); Symbol b = p.symbol("b"); @@ -44,11 +47,38 @@ public void testReplaceOffsetOverOther() strictProject( ImmutableMap.of("a", new ExpressionMatcher("a"), "b", new ExpressionMatcher("b")), filter( - "(row_num > BIGINT '2')", + "row_num > BIGINT '2'", rowNumber( pattern -> pattern .partitionBy(ImmutableList.of()), values("a", "b")) .withAlias("row_num", new RowNumberSymbolMatcher())))); } + + @Test + public void testReplaceOffsetOverSort() + { + tester().assertThat(new ImplementOffset()) + .on(p -> { + Symbol a = p.symbol("a"); + Symbol b = p.symbol("b"); + return p.offset( + 2, + p.sort( + ImmutableList.of(a), + p.values(a, b))); + }) + .matches( + strictProject( + ImmutableMap.of("a", new ExpressionMatcher("a"), "b", new ExpressionMatcher("b")), + filter( + "row_num > BIGINT '2'", + rowNumber( + pattern -> pattern + .partitionBy(ImmutableList.of()), + sort( + ImmutableList.of(sort("a", ASCENDING, FIRST)), + values("a", "b"))) + .withAlias("row_num", new RowNumberSymbolMatcher())))); + } } diff --git a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectSort.java b/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectSort.java deleted file mode 100644 index 9fbd324e81c7..000000000000 --- a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectSort.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.assertions.ExpressionMatcher; -import io.prestosql.sql.planner.assertions.RowNumberSymbolMatcher; -import io.prestosql.sql.planner.iterative.rule.test.BaseRuleTest; -import io.prestosql.sql.planner.plan.Assignments; -import org.testng.annotations.Test; - -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.filter; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.rowNumber; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.sort; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.strictProject; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.values; -import static io.prestosql.sql.tree.SortItem.NullOrdering.FIRST; -import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING; - -public class TestImplementOffsetOverProjectSort - extends BaseRuleTest -{ - @Test - public void testReplaceOffsetOverProjectSort() - { - tester().assertThat(new ImplementOffsetOverProjectSort()) - .on(p -> { - Symbol a = p.symbol("a"); - Symbol b = p.symbol("b"); - return p.offset( - 2, - p.project( - Assignments.identity(b), - p.sort( - ImmutableList.of(a), - p.values(a, b)))); - }) - .matches( - strictProject( - ImmutableMap.of("b", new ExpressionMatcher("b")), - sort( - ImmutableList.of(sort("row_num", ASCENDING, FIRST)), - filter( - "(row_num > BIGINT '2')", - rowNumber( - pattern -> pattern - .partitionBy(ImmutableList.of()), - sort( - ImmutableList.of(sort("a", ASCENDING, FIRST)), - values("a", "b"))) - .withAlias("row_num", new RowNumberSymbolMatcher()))))); - } -} diff --git a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectTopN.java b/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectTopN.java deleted file mode 100644 index 0e3e5ce6558f..000000000000 --- a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverProjectTopN.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.assertions.ExpressionMatcher; -import io.prestosql.sql.planner.assertions.RowNumberSymbolMatcher; -import io.prestosql.sql.planner.iterative.rule.test.BaseRuleTest; -import io.prestosql.sql.planner.plan.Assignments; -import org.testng.annotations.Test; - -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.filter; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.rowNumber; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.sort; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.strictProject; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.topN; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.values; -import static io.prestosql.sql.tree.SortItem.NullOrdering.FIRST; -import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING; - -public class TestImplementOffsetOverProjectTopN - extends BaseRuleTest -{ - @Test - public void testReplaceOffsetOverProjectTopN() - { - tester().assertThat(new ImplementOffsetOverProjectTopN()) - .on(p -> { - Symbol a = p.symbol("a"); - Symbol b = p.symbol("b"); - return p.offset( - 2, - p.project( - Assignments.identity(b), - p.topN( - 5, - ImmutableList.of(a), - p.values(a, b)))); - }) - .matches( - strictProject( - ImmutableMap.of("b", new ExpressionMatcher("b")), - sort( - ImmutableList.of(sort("row_num", ASCENDING, FIRST)), - filter( - "(row_num > BIGINT '2')", - rowNumber( - pattern -> pattern - .partitionBy(ImmutableList.of()), - topN( - 5, - ImmutableList.of(sort("a", ASCENDING, FIRST)), - values("a", "b"))) - .withAlias("row_num", new RowNumberSymbolMatcher()))))); - } -} diff --git a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverSort.java b/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverSort.java deleted file mode 100644 index e2a1c261ec8b..000000000000 --- a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverSort.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.assertions.ExpressionMatcher; -import io.prestosql.sql.planner.assertions.RowNumberSymbolMatcher; -import io.prestosql.sql.planner.iterative.rule.test.BaseRuleTest; -import org.testng.annotations.Test; - -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.filter; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.rowNumber; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.sort; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.strictProject; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.values; -import static io.prestosql.sql.tree.SortItem.NullOrdering.FIRST; -import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING; - -public class TestImplementOffsetOverSort - extends BaseRuleTest -{ - @Test - public void testReplaceOffsetOverSort() - { - tester().assertThat(new ImplementOffsetOverSort()) - .on(p -> { - Symbol a = p.symbol("a"); - Symbol b = p.symbol("b"); - return p.offset( - 2, - p.sort( - ImmutableList.of(a), - p.values(a, b))); - }) - .matches( - strictProject( - ImmutableMap.of("a", new ExpressionMatcher("a"), "b", new ExpressionMatcher("b")), - sort( - ImmutableList.of(sort("row_num", ASCENDING, FIRST)), - filter( - "(row_num > BIGINT '2')", - rowNumber( - pattern -> pattern - .partitionBy(ImmutableList.of()), - sort( - ImmutableList.of(sort("a", ASCENDING, FIRST)), - values("a", "b"))) - .withAlias("row_num", new RowNumberSymbolMatcher()))))); - } -} diff --git a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverTopN.java b/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverTopN.java deleted file mode 100644 index 399eec970860..000000000000 --- a/presto-main/src/test/java/io/prestosql/sql/planner/iterative/rule/TestImplementOffsetOverTopN.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.prestosql.sql.planner.iterative.rule; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.prestosql.sql.planner.Symbol; -import io.prestosql.sql.planner.assertions.ExpressionMatcher; -import io.prestosql.sql.planner.assertions.RowNumberSymbolMatcher; -import io.prestosql.sql.planner.iterative.rule.test.BaseRuleTest; -import org.testng.annotations.Test; - -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.filter; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.rowNumber; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.sort; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.strictProject; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.topN; -import static io.prestosql.sql.planner.assertions.PlanMatchPattern.values; -import static io.prestosql.sql.tree.SortItem.NullOrdering.FIRST; -import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING; - -public class TestImplementOffsetOverTopN - extends BaseRuleTest -{ - @Test - public void testReplaceOffsetOverTopN() - { - tester().assertThat(new ImplementOffsetOverTopN()) - .on(p -> { - Symbol a = p.symbol("a"); - Symbol b = p.symbol("b"); - return p.offset( - 2, - p.topN( - 5, - ImmutableList.of(a), - p.values(a, b))); - }) - .matches( - strictProject( - ImmutableMap.of("a", new ExpressionMatcher("a"), "b", new ExpressionMatcher("b")), - sort( - ImmutableList.of(sort("row_num", ASCENDING, FIRST)), - filter( - "(row_num > BIGINT '2')", - rowNumber( - pattern -> pattern - .partitionBy(ImmutableList.of()), - topN( - 5, - ImmutableList.of(sort("a", ASCENDING, FIRST)), - values("a", "b"))) - .withAlias("row_num", new RowNumberSymbolMatcher()))))); - } -}