more work

marmbrus · marmbrus · commit 34eb3a46efe4 · 2015-02-12T00:01:11.000-08:00
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

Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,7 @@ case class PrettyAttribute(name: String) extends Attribute with trees.LeafNode[E`
`218`	`218`	`override def exprId: ExprId = ???`
`219`	`219`	`override def eval(input: Row): EvaluatedType = ???`
`220`	`220`	`override def nullable: Boolean = ???`
`221`		`- override def dataType: DataType = ???`
	`221`	`+ override def dataType: DataType = NullType`
`222`	`222`	`}`
`223`	`223`
`224`	`224`	`object VirtualColumn {`