Skip to content

[SPARK-16995][SQL] TreeNodeException when flat mapping RelationalGroupedDataset created from DataFrame containing a column created with lit/expr - #14648

Closed
viirya wants to merge 4 commits into
apache:masterfrom
viirya:flat-mapping
Closed

Conversation

@viirya

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

A TreeNodeException is thrown when executing the following minimal example in Spark 2.0.

import spark.implicits._
case class test (x: Int, q: Int)
val d = Seq(1).toDF("x")
d.withColumn("q", lit(0)).as[test].groupByKey(_.x).flatMapGroups{case (x, iter) => List[Int]()}.show
d.withColumn("q", expr("0")).as[test].groupByKey(_.x).flatMapGroups{case (x, iter) => List[Int]()}.show

The problem is at FoldablePropagation. The rule will do transformExpressions on LogicalPlan. The query above contains a MapGroups which has a parameter dataAttributes:Seq[Attribute]. One attributes in dataAttributes will be transformed to an Alias(literal(0), _) in FoldablePropagation. Alias is not an Attribute and causes the error.

We can't easily detect such type inconsistency during transforming expressions. A direct approach to this problem is to skip doing FoldablePropagation on object operators as they should not contain such expressions.

How was this patch tested?

Jenkins tests.

@SparkQA

Copy link
Copy Markdown

Test build #63786 has finished for PR 14648 at commit 2e0d7d6.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya

Copy link
Copy Markdown
MemberAuthor

cc @cloud-fan


// Operators that operate on objects should only have expressions from encoders, which
// should never have foldable expressions.
case o: ObjectConsumer => o

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should follow other cases, to set stop to true

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Ok. I will update once I return back to laptop.

@SparkQA

Copy link
Copy Markdown

Test build #63838 has finished for PR 14648 at commit 4094b43.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya

Copy link
Copy Markdown
MemberAuthor

retest this please.

@SparkQA

Copy link
Copy Markdown

Test build #63845 has finished for PR 14648 at commit 4094b43.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

case o: ObjectProducer =>
stop = true
o
case a: AppendColumns =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After think about it more, I think the foldable propagation is still useful for object operators like DeserializeToObject, how about we specify the problematic cases here explicitly? i.e. MapGroups, FlatMapGroupsInR, CoGroup

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

You mean deserializer in DeserializeToObject can still contain foldable expressions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It can, if the referenced column is a literal.

@SparkQA

Copy link
Copy Markdown

Test build #63892 has finished for PR 14648 at commit 0008c3e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.


import df.sparkSession.implicits._

assertResult(Seq()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not use checkDataset?

@SparkQA

ghost commented Aug 18, 2016

Copy link
Copy Markdown

Test build #63962 has finished for PR 14648 at commit b688abf.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan

ghost commented Aug 18, 2016

Copy link
Copy Markdown
Contributor

thanks, merging to master and 2.0!

@viirya

ghost commented Aug 18, 2016

Copy link
Copy Markdown
MemberAuthor

Thanks for review.

@viirya
viirya deleted the flat-mapping branch December 27, 2023 18:19
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@viirya@SparkQA@cloud-fan