From 2f1b802839b88e3850d3333892fa23127b04486f Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 23 Jul 2015 21:36:04 -0700 Subject: [PATCH 1/3] Add analysis rule to detect sorting on unsupported column types (SPARK-9295) --- .../apache/spark/sql/catalyst/analysis/CheckAnalysis.scala | 4 ++++ .../spark/sql/catalyst/analysis/AnalysisErrorSuite.scala | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala index c203fcecf20f..bc57dc0d79f0 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala @@ -98,6 +98,10 @@ trait CheckAnalysis { aggregateExprs.foreach(checkValidAggregateExpression) + case Sort(order, _, _) if !order.forall(_.dataType.isInstanceOf[AtomicType]) => + val c = order.filterNot(_.dataType.isInstanceOf[AtomicType]).head + failAnalysis(s"Sorting is not supported for columns of type ${c.dataType.simpleString}") + case _ => // Fallbacks to the following checks } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala index dca8c881f21a..83ae6f86ff0d 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala @@ -113,6 +113,11 @@ class AnalysisErrorSuite extends SparkFunSuite with BeforeAndAfter { testRelation.select(Literal(1).cast(BinaryType).as('badCast)), "cannot cast" :: Literal(1).dataType.simpleString :: BinaryType.simpleString :: Nil) + errorTest( + "sorting by unsupported column types", + listRelation.orderBy('list.asc), + "sorting" :: "type" :: "array" :: Nil) + errorTest( "non-boolean filters", testRelation.where(Literal(1)), From bfe1451ec40bcf20f85c5b6fc7bcc1f23bdc6c91 Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 23 Jul 2015 23:18:17 -0700 Subject: [PATCH 2/3] Update to allow sorting by null literals --- .../spark/sql/catalyst/analysis/CheckAnalysis.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala index bc57dc0d79f0..3a76609867a4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala @@ -98,9 +98,14 @@ trait CheckAnalysis { aggregateExprs.foreach(checkValidAggregateExpression) - case Sort(order, _, _) if !order.forall(_.dataType.isInstanceOf[AtomicType]) => - val c = order.filterNot(_.dataType.isInstanceOf[AtomicType]).head - failAnalysis(s"Sorting is not supported for columns of type ${c.dataType.simpleString}") + case Sort(orders, _, _) => + def checkValidSortOrder(order: SortOrder): Unit = order.dataType match { + case t: AtomicType => // OK + case NullType => // OK + case t => + failAnalysis(s"Sorting is not supported for columns of type ${t.simpleString}") + } + orders.foreach(checkValidSortOrder) case _ => // Fallbacks to the following checks } From 23b2fbf1560b481ae61c1e9af2b3403113494f49 Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Fri, 24 Jul 2015 09:48:05 -0700 Subject: [PATCH 3/3] Embed function in foreach --- .../spark/sql/catalyst/analysis/CheckAnalysis.scala | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala index 3a76609867a4..b29490cf889b 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala @@ -99,13 +99,14 @@ trait CheckAnalysis { aggregateExprs.foreach(checkValidAggregateExpression) case Sort(orders, _, _) => - def checkValidSortOrder(order: SortOrder): Unit = order.dataType match { - case t: AtomicType => // OK - case NullType => // OK - case t => - failAnalysis(s"Sorting is not supported for columns of type ${t.simpleString}") + orders.foreach { order => + order.dataType match { + case t: AtomicType => // OK + case NullType => // OK + case t => + failAnalysis(s"Sorting is not supported for columns of type ${t.simpleString}") + } } - orders.foreach(checkValidSortOrder) case _ => // Fallbacks to the following checks }