Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CheckAnalysis {
s"filter expression '${f.condition.prettyString}' " +
s"of type ${f.condition.dataType.simpleString} is not a boolean.")

case aggregatePlan@Aggregate(groupingExprs, aggregateExprs, child) =>
case Aggregate(groupingExprs, aggregateExprs, child) =>
def checkValidAggregateExpression(expr: Expression): Unit = expr match {
case _: AggregateExpression => // OK
case e: Attribute if !groupingExprs.contains(e) =>
Expand All @@ -85,13 +85,18 @@ class CheckAnalysis {

cleaned.foreach(checkValidAggregateExpression)

case _ => // Fallbacks to the following checks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just collapse both into a single pattern match?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because any Aggregate operator must match the two case branches above, and will never match those two for unresolved operator and missing input attribute(s).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok makes sense

}

operator match {
case o if o.children.nonEmpty && o.missingInput.nonEmpty =>
val missingAttributes = o.missingInput.map(_.prettyString).mkString(",")
val input = o.inputSet.map(_.prettyString).mkString(",")
val missingAttributes = o.missingInput.mkString(",")
val input = o.inputSet.mkString(",")

failAnalysis(s"resolved attributes $missingAttributes missing from $input")
failAnalysis(
s"resolved attribute(s) $missingAttributes missing from $input " +
s"in operator ${operator.simpleString}")

// Catch all
case o if !o.resolved =>
failAnalysis(
s"unresolved operator ${operator.simpleString}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,12 @@ abstract class QueryPlan[PlanType <: TreeNode[PlanType]] extends TreeNode[PlanTy
* Attributes that are referenced by expressions but not provided by this nodes children.
* Subclasses should override this method if they produce attributes internally as it is used by
* assertions designed to prevent the construction of invalid plans.
*
* Note that virtual columns should be excluded. Currently, we only support the grouping ID
* virtual column.
*/
def missingInput: AttributeSet = (references -- inputSet)
.filter(_.name != VirtualColumn.groupingIdName)
def missingInput: AttributeSet =
(references -- inputSet).filter(_.name != VirtualColumn.groupingIdName)

/**
* Runs [[transform]] with `rule` on all expressions present in this query operator.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,22 @@ class AnalysisSuite extends FunSuite with BeforeAndAfter {
assert(pl(3).dataType == DecimalType.Unlimited)
assert(pl(4).dataType == DoubleType)
}

test("SPARK-6452 regression test") {
// CheckAnalysis should throw AnalysisException when Aggregate contains missing attribute(s)
val plan =
Aggregate(
Nil,
Alias(Sum(AttributeReference("a", StringType)(exprId = ExprId(1))), "b")() :: Nil,
LocalRelation(
AttributeReference("a", StringType)(exprId = ExprId(2))))

assert(plan.resolved)

val message = intercept[AnalysisException] {
caseSensitiveAnalyze(plan)
}.getMessage

assert(message.contains("resolved attribute(s) a#1 missing from a#2"))
}
}