-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-24339][SQL] Prunes the unused columns from child of ScriptTransformation #21839
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
Conversation
|
@gatorsmile @maropu This is the follow up PR for #21447, please have a look when you have time, thanks. |
|
Test build #93413 has finished for PR 21839 at commit
|
|
Thanks for the work! I’ll review this in hours. |
|
LGTM cc: @gatorsmile |
| case e @ Expand(_, _, child) if (child.outputSet -- e.references).nonEmpty => | ||
| e.copy(child = prunedChild(child, e.references)) | ||
| case s @ ScriptTransformation(_, _, _, child, _) | ||
| if (child.outputSet -- s.references).nonEmpty => |
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.
Nit: two more spaces before if.
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.
Thanks, fix in 2cf131f.
|
@xuanyuanking Could you add an end-to-end test into ScriptTransformationSuite to verify the results? |
|
@gatorsmile Thanks for your advice, added ut in ScriptTransformationSuite. |
|
LGTM |
|
Test build #93448 has finished for PR 21839 at commit
|
|
Thanks! Merged to master. |
|
Thanks for reviewing. |
What changes were proposed in this pull request?
Modify the strategy in ColumnPruning to add a Project between ScriptTransformation and its child, this strategy can reduce the scan time especially in the scenario of the table has many columns.
How was this patch tested?
Add UT in ColumnPruningSuite and ScriptTransformationSuite.