-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Support Accumulator
for avg duration
#15468
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
Ping @shruti2522 -- this one looks pretty close |
I think you need to update the signature of Avg to support the new type as well |
051e33b
to
bb16e43
Compare
962cbfc
to
a986491
Compare
@alamb this one's ready for review whenever you get a chance. |
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 very much @shruti2522 -- this code looks really nice. I think we need a few more tests but then it will be ready to merge
Thank you so much. This is great
@@ -4969,6 +4969,25 @@ select count(distinct column1), count(distinct column2) from dict_test group by | |||
statement ok | |||
drop table dict_test; | |||
|
|||
# avg_duartion |
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.
# avg_duartion | |
# avg_duration |
b28ac9f
to
8abc5b9
Compare
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 @shruti2522
* Support Accumulator for avg duration * Add tests
Which issue does this PR close?
AVG
for durations #15458Rationale for this change
Enable support for averaging
Duration
typeWhat changes are included in this PR?
Added accumulator to compute the Avg of
Duration
types, would add support forGroupsAccumulator
in a follow-up PR.Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.