-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-12288] [SQL] Support UnsafeRow in Coalesce/Except/Intersect. #10285
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
Changes from all commits
cbb2c59
f2f4b48
b17ba61
b3b70af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,41 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext { | |
| assert(!preparedPlan.outputsUnsafeRows) | ||
| } | ||
|
|
||
| test("coalesce can process unsafe rows") { | ||
| val plan = Coalesce(1, outputsUnsafe) | ||
| val preparedPlan = sqlContext.prepareForExecution.execute(plan) | ||
| assert(getConverters(preparedPlan).size === 1) | ||
| assert(preparedPlan.outputsUnsafeRows) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we add mixed rows test for coalesce too?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I am not sure if I understand your question.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh sorry, I thought it was
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! |
||
|
|
||
| test("except can process unsafe rows") { | ||
| val plan = Except(outputsUnsafe, outputsUnsafe) | ||
| val preparedPlan = sqlContext.prepareForExecution.execute(plan) | ||
| assert(getConverters(preparedPlan).size === 2) | ||
| assert(preparedPlan.outputsUnsafeRows) | ||
| } | ||
|
|
||
| test("except requires all of its input rows' formats to agree") { | ||
| val plan = Except(outputsSafe, outputsUnsafe) | ||
| assert(plan.canProcessSafeRows && plan.canProcessUnsafeRows) | ||
| val preparedPlan = sqlContext.prepareForExecution.execute(plan) | ||
| assert(preparedPlan.outputsUnsafeRows) | ||
| } | ||
|
|
||
| test("intersect can process unsafe rows") { | ||
| val plan = Intersect(outputsUnsafe, outputsUnsafe) | ||
| val preparedPlan = sqlContext.prepareForExecution.execute(plan) | ||
| assert(getConverters(preparedPlan).size === 2) | ||
| assert(preparedPlan.outputsUnsafeRows) | ||
| } | ||
|
|
||
| test("intersect requires all of its input rows' formats to agree") { | ||
| val plan = Intersect(outputsSafe, outputsUnsafe) | ||
| assert(plan.canProcessSafeRows && plan.canProcessUnsafeRows) | ||
| val preparedPlan = sqlContext.prepareForExecution.execute(plan) | ||
| assert(preparedPlan.outputsUnsafeRows) | ||
| } | ||
|
|
||
| test("execute() fails an assertion if inputs rows are of different formats") { | ||
| val e = intercept[AssertionError] { | ||
| Union(Seq(outputsSafe, outputsUnsafe)).execute() | ||
|
|
||
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.
UnsafeRow can be compared with GenericInternalRow, so we should make sure that
leftandrighthave the same type of row.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.
seems it's already promised in https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/rowFormatConverters.scala#L95-L106
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.
Thank you! @davies @cloud-fan I added two extra test cases for ensuring they have the same formats.