-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Minor: include sort
expressions in SortPreservingRepartitionExec
explain plan
#7796
Conversation
d49d32f
to
f5210af
Compare
write!( | ||
f, | ||
"SortRequiredExec: [{}]", | ||
PhysicalSortExpr::format_list(&self.expr) |
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.
this is a cleanup to use the same code everywhere to format [PhysicalSortExpr]
) | ||
)?; | ||
|
||
if let Some(sort_exprs) = self.sort_exprs() { |
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.
this is the new behavior
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 @alamb for this PR. It is LGTM!.
Which issue does this PR close?
Related to #7794
Rationale for this change
A plan without sort exprs is incorrect, but it was not clear to me this was happening during my initial debugging. I would like to make it easier to debug such errors in the future
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?