Skip to content

Conversation

hawkfish
Copy link
Contributor

Implement AVG for all temporal types (except INTERVAL and TIME_TZ...)

Implement AVG for all temporal types (except INTERVAL and TIME_TZ...)
Copy link
Contributor

@lnkuiper lnkuiper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! LGTM, I just have one question about the rounding. I guess it might be an arbitrary choice, but why do we want to round up? Do any other systems implement avg for temporals and also round up instead of down? Maybe MySQL?

@hawkfish
Copy link
Contributor Author

Thanks for the PR! LGTM, I just have one question about the rounding. I guess it might be an arbitrary choice, but why do we want to round up? Do any other systems implement avg for temporals and also round up instead of down? Maybe MySQL?

I just added rounding because we always do that for fixed point division/casting. It's only ±1µs so not a big deal either way.

I can't find any other system that does this (Oracle, MS-SQL, ...) - although there are lots of people asking and being given workarounds that involve casting to a numeric value. As the PR shows, it's not difficult, but no one seems to do it.

(Funny factoid about MySQL dates: Because they support zeros for day and month fields to mean "all month" and "all year", their timestamps are not dense numbers, so there is no way to average their date values!)

@Mytherin Mytherin merged commit eba8c65 into duckdb:main Feb 6, 2025
53 checks passed
@hawkfish hawkfish deleted the temporal-avg branch February 10, 2025 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants