-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
Conversation
) | ||
|
||
val sortOrder = Seq( | ||
SortOrder(BoundReference(0, StringType, nullable = false), Ascending), |
There was a problem hiding this comment.
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.
Test build #35149 has finished for PR 6885 at commit
|
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@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? |
+1 on backporting to branch-1.4 |
Test build #35177 has finished for PR 6885 at commit
|
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. |
Test build #35174 has finished for PR 6885 at commit
|
LGTM |
…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>
Thanks! Merged to master and branch-1.4 |
…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
This patch introduces
SparkPlanTest
, a base class for unit tests of SparkPlan physical operators. This is analogous to Spark SQL's existingQueryTest
, 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.