-
Notifications
You must be signed in to change notification settings - Fork 21
Improve docs and add custom execution plan example #277
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
Improve docs and add custom execution plan example #277
Conversation
d849092 to
39a9509
Compare
10b13f3 to
d2241c0
Compare
7adbc9c to
84e6d8a
Compare
84e6d8a to
f2b4fed
Compare
|
Looks good to me, just made a few wording suggestions in the docs. I like the code example you came up with, I think it highlights the behavior of 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.
Is NetworkCoalesceExec at the top not considered a stage? Do stage indices start at 0 or 1? It could possibly be helpful to make that explicit by writing e.g. Stage i where i is the stage index rather than just stage.
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.
Is NetworkCoalesceExec at the top not considered a stage?
I don't think so. That part of the plan gets executed locally in a non-distributed context, so we usually talk about it as the "head" of the plan. It might be nice to choose a specific term for that though.
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.
For stage enumeration, I'd not over index to much about it in this drawing, as I wanted to land it as clean as possible. There's not a strong enumeration order requirement, fwiw stage numbers could be just random numbers and it would work the same, so I think it's fine to not represent it in this drawing
NGA-TRAN
left a comment
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.
@JSOD11 has great comments. I only wonder whether we can also display the plans with the examples
This is a docs-specific PR that: