fixes trinodb#759

presto-main/src/main/java/io/prestosql/sql/planner/PlanOptimizers.java

@@ -136,6 +136,7 @@
 import io.prestosql.sql.planner.optimizations.PlanOptimizer;
 import io.prestosql.sql.planner.optimizations.PredicatePushDown;
 import io.prestosql.sql.planner.optimizations.PruneUnreferencedOutputs;
+import io.prestosql.sql.planner.optimizations.RemoveRedundantOrderByOptimizer;
 import io.prestosql.sql.planner.optimizations.ReplicateSemiJoinInDelete;
 import io.prestosql.sql.planner.optimizations.SetFlatteningOptimizer;
 import io.prestosql.sql.planner.optimizations.StatsRecordingPlanOptimizer;
@@ -492,6 +493,8 @@ public PlanOptimizers(
                         .add(new InlineProjections())
                         .build()));
 
+        builder.add(new RemoveRedundantOrderByOptimizer());
+
         if (!forceSingleNode) {
             builder.add(new ReplicateSemiJoinInDelete()); // Must run before AddExchanges
             builder.add((new IterativeOptimizer(

presto-main/src/main/java/io/prestosql/sql/planner/optimizations/RemoveRedundantOrderByOptimizer.java

@@ -0,0 +1,107 @@
+/*
+ * 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.optimizations;
+
+import io.airlift.log.Logger;
+import io.prestosql.Session;
+import io.prestosql.execution.warnings.WarningCollector;
+import io.prestosql.sql.planner.PlanNodeIdAllocator;
+import io.prestosql.sql.planner.SymbolAllocator;
+import io.prestosql.sql.planner.TypeProvider;
+import io.prestosql.sql.planner.plan.AggregationNode;
+import io.prestosql.sql.planner.plan.ChildReplacer;
+import io.prestosql.sql.planner.plan.JoinNode;
+import io.prestosql.sql.planner.plan.LimitNode;
+import io.prestosql.sql.planner.plan.OutputNode;
+import io.prestosql.sql.planner.plan.PlanNode;
+import io.prestosql.sql.planner.plan.SimplePlanRewriter;
+import io.prestosql.sql.planner.plan.SortNode;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static io.prestosql.sql.planner.plan.SimplePlanRewriter.rewriteWith;
+
+public class RemoveRedundantOrderByOptimizer
+        implements PlanOptimizer
+{
+    private static final Logger logger = Logger.get(RemoveRedundantOrderByOptimizer.class);
+
+    @Override
+    public PlanNode optimize(PlanNode plan, Session session, TypeProvider types, SymbolAllocator symbolAllocator, PlanNodeIdAllocator idAllocator, WarningCollector warningCollector)
+    {
+        OrderByContext orderByContext = new OrderByContext(true);
+        return rewriteWith(new Rewriter(), plan, orderByContext);
+    }
+
+    private class Rewriter
+            extends SimplePlanRewriter<OrderByContext>
+    {
+        @Override
+        public PlanNode visitOutput(OutputNode node, RewriteContext<OrderByContext> context)
+        {
+            //Setting orderingUsedAbove to true
+            return context.defaultRewrite(node, new OrderByContext(true));
+        }
+
+        protected PlanNode visitPlan(PlanNode node, RewriteContext<OrderByContext> context)
+        {
+            boolean orderingUsedAbove = context.get().isOrderingUsedAbove();
+            if (node instanceof JoinNode || node instanceof AggregationNode || node instanceof LimitNode) {
+                orderingUsedAbove = false;
+                context.defaultRewrite(node, new OrderByContext(false));
+            }
+            else {
+                context.defaultRewrite(node, context.get());
+            }
+
+            if (!orderingUsedAbove && !node.getSources().isEmpty()) {
+                List<PlanNode> newChildren = null;
+                for (int i = 0; i < node.getSources().size(); i++) {
+                    if (node.getSources().get(i) instanceof SortNode) {
+                        if (newChildren == null) {
+                            newChildren = new ArrayList(node.getSources());
+                        }
+                        newChildren.set(i, ((SortNode) node.getSources().get(i)).getSource());
+                    }
+                }
+                if (newChildren != null) {
+                    return ChildReplacer.replaceChildren(node, newChildren);
+                }
+            }
+            return node;
+        }
+    }
+
+    private class OrderByContext
+    {
+        boolean orderingUsedAbove;
+
+        public OrderByContext(boolean orderingUsedAbove)
+        {
+            this.orderingUsedAbove = orderingUsedAbove;
+        }
+
+        public boolean isOrderingUsedAbove()
+        {
+            return orderingUsedAbove;
+        }
+
+        public OrderByContext setOrderingUsedAbove(boolean orderingUsedAbove)
+        {
+            this.orderingUsedAbove = orderingUsedAbove;
+            return this;
+        }
+    }
+}

presto-main/src/test/java/io/prestosql/sql/planner/optimizations/TestRemoveRedundantOrderBy.java

@@ -0,0 +1,63 @@
+/*
+ * 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.optimizations;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import io.prestosql.sql.planner.assertions.BasePlanTest;
+import io.prestosql.sql.planner.assertions.PlanMatchPattern;
+import org.testng.annotations.Test;
+
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.anyTree;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.equiJoinClause;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.exchange;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.join;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.project;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.sort;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.tableScan;
+import static io.prestosql.sql.planner.assertions.PlanMatchPattern.topN;
+import static io.prestosql.sql.planner.plan.JoinNode.Type.INNER;
+import static io.prestosql.sql.tree.SortItem.NullOrdering.LAST;
+import static io.prestosql.sql.tree.SortItem.Ordering.ASCENDING;
+
+public class TestRemoveRedundantOrderBy
+        extends BasePlanTest
+{
+    private static final PlanMatchPattern ORDERS_TABLESCAN = tableScan("orders", ImmutableMap.of("O_ORDERKEY", "orderkey"));
+    private static final PlanMatchPattern LINEITEM_TABLESCAN = tableScan(
+            "lineitem",
+            ImmutableMap.of(
+                    "L_PARTKEY", "partkey",
+                    "L_ORDERKEY", "orderkey"));
+
+    @Test
+    public void testRemoveRedundantOrderBy()
+    {
+        String sql1 = "select * from orders a join (select * from lineitem order by orderkey) b on a.orderkey = b.orderkey";
+        assertPlan(sql1,
+                anyTree(join(INNER, ImmutableList.of(equiJoinClause("O_ORDERKEY", "L_ORDERKEY")),
+                        project(tableScan("orders")),
+                        exchange(project(tableScan("lineitem"))))));
+    }
+
+    @Test
+    public void testOrderByWithLimitNotRemoved()
+    {
+        String sql1 = "select * from orders a join (select * from lineitem order by orderkey limit 10) b on a.orderkey = b.orderkey";
+        assertPlan(sql1,
+                anyTree(join(INNER, ImmutableList.of(equiJoinClause("O_ORDERKEY", "L_ORDERKEY")),
+                        project(tableScan("orders")),
+                        topN(10, ImmutableList.of(sort("l_orderkey", ASCENDING, LAST)), exchange(project(anyTree(LINEITEM_TABLESCAN)))))));
+    }
+}