-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: Add reduction_factor metric to AggregateExec for EXPLAIN ANALYZE
#18455
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
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! Good to go after CI passes.
Co-authored-by: Yongting You <[email protected]>
|
Oh my bad, I didn't mean to click re-request review. (I forgot this is datafusion with the 24 hr merge delay) |
| assert_metrics!( | ||
| &formatted, | ||
| "AggregateExec: mode=Partial, gby=[c1@0 as c1]", | ||
| "reduction_factor=5.1% (5/99)" |
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 really nice
|
Thank you @2010YOUY01 and @petern48 |
Which issue does this PR close?
reduction_fatormetric (forEXPLAIN ANALYZE) inAggregateExec#18410Rationale for this change
What changes are included in this PR?
This PR adds the
reduction_factormetric to theAggregateExecmode=Partial case.e.g from the issue
Note: For AggregateExec cases where this doesn't apply, the reduction_factor metric won't be shown. Here's an example of the explain analyze from the modified test in
explain_analyze.rs.The following cases don't include
reduction_factormetricAggregateExec: mode=Final, gby=[], aggr=[count(Int64(1))]AggregateExec: mode=Partial, gby=[], aggr=[count(Int64(1))]AggregateExec: mode=FinalPartitioned, gby=[c1@0 as c1], aggr=[]While this case does:
AggregateExec: mode=Partial, gby=[c1@0 as c1], aggr=[]->reduction_factor=5.1% (5/99)Are these changes tested?
Yes
Are there any user-facing changes?
Yes, a new metric will be visible when running
EXPLAIN ANALYZE