Skip to content

Commit

Permalink
[SPARK-13694][SQL] QueryPlan.expressions should always include all ex…
Browse files Browse the repository at this point in the history
…pressions

## What changes were proposed in this pull request?

It's weird that expressions don't always have all the expressions in it. This PR marks `QueryPlan.expressions` final to forbid sub classes overriding it to exclude some expressions. Currently only `Generate` override it, we can use `producedAttributes` to fix the unresolved attribute problem for it.

Note that this PR doesn't fix the problem in #11497

## How was this patch tested?

existing tests.

Author: Wenchen Fan <wenchen@databricks.com>

Closes #11532 from cloud-fan/generate.
  • Loading branch information
cloud-fan authored and marmbrus committed Mar 7, 2016
1 parent d7eac9d commit 4896411
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ abstract class QueryPlan[PlanType <: TreeNode[PlanType]] extends TreeNode[PlanTy
}

/** Returns all of the expressions present in this query plan operator. */
def expressions: Seq[Expression] = {
final def expressions: Seq[Expression] = {
// Recursively find all expressions from a traversable.
def seqToExpressions(seq: Traversable[Any]): Traversable[Expression] = seq.flatMap {
case e: Expression => e :: Nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ case class Generate(
generatorOutput.forall(_.resolved)
}

// we don't want the gOutput to be taken as part of the expressions
// as that will cause exceptions like unresolved attributes etc.
override def expressions: Seq[Expression] = generator :: Nil
override def producedAttributes: AttributeSet = AttributeSet(generatorOutput)

def output: Seq[Attribute] = {
val qualified = qualifier.map(q =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,6 @@ case class CoGroup(
left: LogicalPlan,
right: LogicalPlan) extends BinaryNode with ObjectOperator {

override def producedAttributes: AttributeSet = outputSet

override def deserializers: Seq[(Expression, Seq[Attribute])] =
// The `leftGroup` and `rightGroup` are guaranteed te be of same schema, so it's safe to resolve
// the `keyDeserializer` based on either of them, here we pick the left one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ case class Generate(
private[sql] override lazy val metrics = Map(
"numOutputRows" -> SQLMetrics.createLongMetric(sparkContext, "number of output rows"))

override def expressions: Seq[Expression] = generator :: Nil
override def producedAttributes: AttributeSet = AttributeSet(output)

val boundGenerator = BindReferences.bindReference(generator, child.output)

Expand Down

0 comments on commit 4896411

Please sign in to comment.