From 96cd8ce0e7b3729483a21b73b9c54c480d627ab7 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Fri, 18 Mar 2016 22:19:16 -0700 Subject: [PATCH 1/4] PruneSorts --- .../sql/catalyst/optimizer/Optimizer.scala | 12 ++++ .../catalyst/optimizer/PruneSortsSuite.scala | 66 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala index c419b5fd2204b..b2e97e0a59a7c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala @@ -87,6 +87,7 @@ abstract class Optimizer extends RuleExecutor[LogicalPlan] { SimplifyConditionals, RemoveDispensableExpressions, PruneFilters, + PruneSorts, SimplifyCasts, SimplifyCaseConversionExpressions, EliminateSerialization) :: @@ -825,6 +826,17 @@ object CombineFilters extends Rule[LogicalPlan] with PredicateHelper { } } +/** + * Removes no-op SortOrder from Sort + */ +object PruneSorts extends Rule[LogicalPlan] { + def apply(plan: LogicalPlan): LogicalPlan = plan transform { + case s @ Sort(orders, _, child) if orders.exists(_.references.isEmpty) || orders.isEmpty => + val newOrder = orders.filter(_.references.nonEmpty) + if (newOrder.isEmpty) child else s.copy(order = newOrder) + } +} + /** * Removes filters that can be evaluated trivially. This can be done through the following ways: * 1) by eliding the filter for cases where it will always evaluate to `true`. diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala new file mode 100644 index 0000000000000..1d4b3102a19af --- /dev/null +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.spark.sql.catalyst.optimizer + +import org.apache.spark.sql.catalyst.dsl.expressions._ +import org.apache.spark.sql.catalyst.dsl.plans._ +import org.apache.spark.sql.catalyst.expressions._ +import org.apache.spark.sql.catalyst.plans._ +import org.apache.spark.sql.catalyst.plans.logical._ +import org.apache.spark.sql.catalyst.rules._ + +class PruneSortsSuite extends PlanTest { + + object Optimize extends RuleExecutor[LogicalPlan] { + val batches = + Batch("Prune Sorts", Once, + PruneSorts) :: Nil + } + + val testRelation = LocalRelation('a.int, 'b.int, 'c.int) + + test("Empty order by clause") { + val x = testRelation + + val query = x.orderBy() + val optimized = Optimize.execute(query.analyze) + val correctAnswer = x.analyze + + comparePlans(optimized, correctAnswer) + } + + test("All the SortOrder are no-op") { + val x = testRelation + + val query = x.orderBy(SortOrder(3, Ascending), SortOrder(-1, Ascending)) + val optimized = Optimize.execute(query.analyze) + val correctAnswer = x.analyze + + comparePlans(optimized, correctAnswer) + } + + test("Partial order-by clauses contain no-op SortOrder") { + val x = testRelation + + val query = x.orderBy(SortOrder(3, Ascending), 'a.asc) + val optimized = Optimize.execute(query.analyze) + val correctAnswer = x.orderBy('a.asc).analyze + + comparePlans(optimized, correctAnswer) + } +} From 0cc3f4ff7b2db82f9d929a6683f7e32bfd135fb5 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Fri, 18 Mar 2016 23:49:48 -0700 Subject: [PATCH 2/4] address comments. --- .../spark/sql/catalyst/optimizer/Optimizer.scala | 10 +++++----- ...PruneSortsSuite.scala => EliminateSortsSuite.scala} | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) rename sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/{PruneSortsSuite.scala => EliminateSortsSuite.scala} (94%) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala index b2e97e0a59a7c..347a044004788 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala @@ -87,7 +87,7 @@ abstract class Optimizer extends RuleExecutor[LogicalPlan] { SimplifyConditionals, RemoveDispensableExpressions, PruneFilters, - PruneSorts, + EliminateSorts, SimplifyCasts, SimplifyCaseConversionExpressions, EliminateSerialization) :: @@ -829,11 +829,11 @@ object CombineFilters extends Rule[LogicalPlan] with PredicateHelper { /** * Removes no-op SortOrder from Sort */ -object PruneSorts extends Rule[LogicalPlan] { +object EliminateSorts extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan transform { - case s @ Sort(orders, _, child) if orders.exists(_.references.isEmpty) || orders.isEmpty => - val newOrder = orders.filter(_.references.nonEmpty) - if (newOrder.isEmpty) child else s.copy(order = newOrder) + case s @ Sort(orders, _, child) if orders.isEmpty || orders.exists(_.child.foldable) => + val newOrders = orders.filter(_.references.nonEmpty) + if (newOrders.isEmpty) child else s.copy(order = newOrders) } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/EliminateSortsSuite.scala similarity index 94% rename from sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala rename to sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/EliminateSortsSuite.scala index 1d4b3102a19af..27c15e856aba5 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PruneSortsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/EliminateSortsSuite.scala @@ -24,12 +24,12 @@ import org.apache.spark.sql.catalyst.plans._ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules._ -class PruneSortsSuite extends PlanTest { +class EliminateSortsSuite extends PlanTest { object Optimize extends RuleExecutor[LogicalPlan] { val batches = - Batch("Prune Sorts", Once, - PruneSorts) :: Nil + Batch("Eliminate Sorts", Once, + EliminateSorts) :: Nil } val testRelation = LocalRelation('a.int, 'b.int, 'c.int) From de2b624e391678ae3661985fc9ef571ba6b0302f Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Sat, 19 Mar 2016 00:08:50 -0700 Subject: [PATCH 3/4] address comments. --- .../org/apache/spark/sql/catalyst/optimizer/Optimizer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala index 347a044004788..ae45a743b926e 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala @@ -832,7 +832,7 @@ object CombineFilters extends Rule[LogicalPlan] with PredicateHelper { object EliminateSorts extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan transform { case s @ Sort(orders, _, child) if orders.isEmpty || orders.exists(_.child.foldable) => - val newOrders = orders.filter(_.references.nonEmpty) + val newOrders = orders.filter(!_.child.foldable) if (newOrders.isEmpty) child else s.copy(order = newOrders) } } From 6ebccc70130943fdb9c8b99af8ab81b44e2ba0bb Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Sat, 19 Mar 2016 00:09:51 -0700 Subject: [PATCH 4/4] address comments. --- .../org/apache/spark/sql/catalyst/optimizer/Optimizer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala index ae45a743b926e..41e8dc0f46746 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala @@ -832,7 +832,7 @@ object CombineFilters extends Rule[LogicalPlan] with PredicateHelper { object EliminateSorts extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan transform { case s @ Sort(orders, _, child) if orders.isEmpty || orders.exists(_.child.foldable) => - val newOrders = orders.filter(!_.child.foldable) + val newOrders = orders.filterNot(_.child.foldable) if (newOrders.isEmpty) child else s.copy(order = newOrders) } }