more work

marmbrus · Feb 12, 2015 · 34eb3a4 · 34eb3a4
1 parent 6197cd5
commit 34eb3a4
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 12 deletions.
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
@@ -83,7 +83,7 @@ class Analyzer(catalog: Catalog,
           operator transformAllExpressions {
             case a: Attribute if !a.resolved =>
               val from = operator.inputSet.map(_.name).mkString("{", ", ", "}")
-              failAnalysis(s"cannot resolve '$a' given input columns $from")
+              failAnalysis(s"cannot resolve '${a.prettyString}' given input columns $from")
 
             case c: Cast if !c.resolved =>
               failAnalysis(
@@ -93,13 +93,13 @@ class Analyzer(catalog: Catalog,
               failAnalysis(
                 s"invalid expression ${b.prettyString} " +
                 s"between ${b.left.simpleString} and ${b.right.simpleString}")
-
-
           }
 
           operator match {
             case f: Filter if f.condition.dataType != BooleanType =>
-              failAnalysis(s"filter expression '${f.condition.prettyString}' is not a boolean.")
+              failAnalysis(
+                s"filter expression '${f.condition.prettyString}' " +
+                s"of type ${f.condition.dataType.simpleString} is not a boolean.")
 
             case aggregatePlan @ Aggregate(groupingExprs, aggregateExprs, child) =>
               def isValidAggregateExpression(expr: Expression): Boolean = expr match {
@@ -118,11 +118,10 @@ class Analyzer(catalog: Catalog,
                   case Alias(g: GetField, _) => g
                 })
               }.foreach { e =>
-                failAnalysis(s"expression must be aggregates or be in group by $e")
+                failAnalysis(
+                  s"expression '${e.prettyString}' is not an aggregate function or in the group by")
               }
 
-              aggregatePlan
-
             case o if o.children.nonEmpty && !o.references.subsetOf(o.inputSet) =>
               val missingAttributes = (o.references -- o.inputSet).map(_.prettyString).mkString(",")
               val input = o.inputSet.map(_.prettyString).mkString(",")

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala
@@ -218,7 +218,7 @@ case class PrettyAttribute(name: String) extends Attribute with trees.LeafNode[E
   override def exprId: ExprId = ???
   override def eval(input: Row): EvaluatedType = ???
   override def nullable: Boolean = ???
-  override def dataType: DataType = ???
+  override def dataType: DataType = NullType
 }
 
 object VirtualColumn {

diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
@@ -136,6 +136,17 @@ class AnalysisSuite extends FunSuite with BeforeAndAfter {
     testRelation.select(Literal(1).cast(BinaryType).as('badCast)),
     "invalid cast" :: Literal(1).dataType.simpleString :: BinaryType.simpleString :: Nil)
 
+  errorTest(
+    "non-boolean filters",
+    testRelation.where(Literal(1)),
+    "filter" :: "'1'" :: "not a boolean" :: Literal(1).dataType.simpleString :: Nil)
+
+  errorTest(
+    "missing group by",
+    testRelation2.groupBy('a)('b),
+    "'b'" :: "group by" :: Nil
+  )
+
   case class UnresolvedTestPlan() extends LeafNode {
     override lazy val resolved = false
     override def output = Nil

diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
@@ -806,10 +806,8 @@ class SQLQuerySuite extends QueryTest with BeforeAndAfterAll {
   test("throw errors for non-aggregate attributes with aggregation") {
     def checkAggregation(query: String, isInvalidQuery: Boolean = true) {
       if (isInvalidQuery) {
-        val e = intercept[TreeNodeException[LogicalPlan]](sql(query).queryExecution.analyzed)
-        assert(
-          e.getMessage.startsWith("Expression not in GROUP BY"),
-          "Non-aggregate attribute(s) not detected\n")
+        val e = intercept[AnalysisException](sql(query).queryExecution.analyzed)
+        assert(e.getMessage contains "group by")
       } else {
         // Should not throw
         sql(query).queryExecution.analyzed