Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-8446] [SQL] Add helper functions for testing SparkPlan physical operators #6885

Closed
wants to merge 8 commits into from

Conversation

JoshRosen
Copy link
Contributor

This patch introduces SparkPlanTest, a base class for unit tests of SparkPlan physical operators. This is analogous to Spark SQL's existing QueryTest, which does something similar for end-to-end tests with actual queries.

These helper methods provide nicer error output when tests fail and help developers to avoid writing lots of boilerplate in order to execute manually constructed physical plans.

)

val sortOrder = Seq(
SortOrder(BoundReference(0, StringType, nullable = false), Ascending),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little annoying to have to manually bind these references. It would be nice if there was some sort of rewrite that I could use that would bind references like '_1 to the proper columns.

@SparkQA
Copy link

SparkQA commented Jun 18, 2015

Test build #35149 has finished for PR 6885 at commit c60a44d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class SortSuite extends SparkPlanTest
    • class SparkPlanTest extends SparkFunSuite

val converted: Seq[Row] = answer.map { s =>
Row.fromSeq(s.toSeq.map {
case d: java.math.BigDecimal => BigDecimal(d)
case b: Array[Byte] => b.toSeq
Copy link
Contributor

Choose a reason for hiding this comment

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

The equality check of Array[Byte] will be fixed by #6876

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, gotcha. I'd be happy to block on this. One consideration, though: we might want to backport this test helper to some of our maintenance branches so that we don't get build failures when backporting regression tests which use SparkPlanTest. In that case, we might also need to backport those other byte comparison fixes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of blocking lets just make a note in the JIRA to remove these hacks if possible later. I'd like to get this in today and I agree with you backporting concerns.

@JoshRosen
Copy link
Contributor Author

@marmbrus, I've updated this patch based on your PR and have removed a bit of dead code. What are your thoughts on backporting this to at least 1.4 so that we can use it when writing regression tests?

@marmbrus
Copy link
Contributor

+1 on backporting to branch-1.4

@SparkQA
Copy link

SparkQA commented Jun 18, 2015

Test build #35177 has finished for PR 6885 at commit f8ce275.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class SortSuite extends SparkPlanTest
    • class SparkPlanTest extends SparkFunSuite

@JoshRosen
Copy link
Contributor Author

That Python test failure looks like it's caused by a flaky test or an environment issue, not any changes in this patch.

@marmbrus, did you have any other review comments or is this good to merge? If you'd like, I can handle the backport cherry-pick.

@SparkQA
Copy link

SparkQA commented Jun 18, 2015

Test build #35174 has finished for PR 6885 at commit a80f9b0.

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

@marmbrus
Copy link
Contributor

LGTM

@asfgit asfgit closed this in 207a98c Jun 18, 2015
asfgit pushed a commit that referenced this pull request Jun 18, 2015
…l operators

This patch introduces `SparkPlanTest`, a base class for unit tests of SparkPlan physical operators.  This is analogous to Spark SQL's existing `QueryTest`, which does something similar for end-to-end tests with actual queries.

These helper methods provide nicer error output when tests fail and help developers to avoid writing lots of boilerplate in order to execute manually constructed physical plans.

Author: Josh Rosen <joshrosen@databricks.com>
Author: Josh Rosen <rosenville@gmail.com>
Author: Michael Armbrust <michael@databricks.com>

Closes #6885 from JoshRosen/spark-plan-test and squashes the following commits:

f8ce275 [Josh Rosen] Fix some IntelliJ inspections and delete some dead code
84214be [Josh Rosen] Add an extra column which isn't part of the sort
ae1896b [Josh Rosen] Provide implicits automatically
a80f9b0 [Josh Rosen] Merge pull request #4 from marmbrus/pr/6885
d9ab1e4 [Michael Armbrust] Add simple resolver
c60a44d [Josh Rosen] Manually bind references
996332a [Josh Rosen] Add types so that tests compile
a46144a [Josh Rosen] WIP

(cherry picked from commit 207a98c)
Signed-off-by: Michael Armbrust <michael@databricks.com>
@marmbrus
Copy link
Contributor

Thanks! Merged to master and branch-1.4

@JoshRosen JoshRosen deleted the spark-plan-test branch June 18, 2015 23:56
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
…l operators

This patch introduces `SparkPlanTest`, a base class for unit tests of SparkPlan physical operators.  This is analogous to Spark SQL's existing `QueryTest`, which does something similar for end-to-end tests with actual queries.

These helper methods provide nicer error output when tests fail and help developers to avoid writing lots of boilerplate in order to execute manually constructed physical plans.

Author: Josh Rosen <joshrosen@databricks.com>
Author: Josh Rosen <rosenville@gmail.com>
Author: Michael Armbrust <michael@databricks.com>

Closes apache#6885 from JoshRosen/spark-plan-test and squashes the following commits:

f8ce275 [Josh Rosen] Fix some IntelliJ inspections and delete some dead code
84214be [Josh Rosen] Add an extra column which isn't part of the sort
ae1896b [Josh Rosen] Provide implicits automatically
a80f9b0 [Josh Rosen] Merge pull request apache#4 from marmbrus/pr/6885
d9ab1e4 [Michael Armbrust] Add simple resolver
c60a44d [Josh Rosen] Manually bind references
996332a [Josh Rosen] Add types so that tests compile
a46144a [Josh Rosen] WIP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants