-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
remove hard coded partition count in ballista logicalplan deserialization #1044
Conversation
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.
Looks like we only changed the function argument name, so it should be backwards compatible 👍 |
Looks great! I still find the |
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.
I think this looks great -- thank you @xudong963 !
Adding some more doc strings would be useful as @rdettai but I think it can also be done as a follow on PR.
Thanks again for sticking with this
Thanks for your help! |
/// Number of partitions for query execution. Increasing partitions may increase concurrency. Please help me check if the comments are correct and detail @rdettai, I will add it in the next PR. |
I think that the current DataFusion default collection method does start all the tasks in parallel. I will try to go through all the places where the parameter is used to try to synthesize more precisely what it means. |
Which issue does this PR close?
Closes ##972
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?