Skip to content
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

Add integral function for time_weight #526

Merged
merged 3 commits into from
Sep 20, 2022
Merged

Add integral function for time_weight #526

merged 3 commits into from
Sep 20, 2022

Conversation

syvb
Copy link
Member

@syvb syvb commented Sep 7, 2022

Implements functionality requested in #455:

  • Adds an experimental integral(tws[, unit]) -> float8 function (unit defaults to 'second')
  • Adds an experimental interpolated_integral(tws, start, interval, prev, next[, unit]) -> float8 function (unit defaults to 'second')
  • Adds experimental arrow functions for integral/interpolated_integral
  • Makes trapezoidal an alias for linear in the time_weight function

The unit parameter to integral/interpolated_integral is a string that specifies what time unit to use for the returned f64. It can be any fixed-duration unit that PostgreSQL allows in a interval. An alternative would be to have an interval be passed as the unit instead, but intervals can have variable length units (days and months) which might not be wanted. Also, integral(..., 'hour') is clearer than integral(..., '1 hour'::interval).

@syvb syvb force-pushed the smittyvb/integral branch 2 times, most recently from 6882a6f to daefd4b Compare September 9, 2022 14:42
@syvb syvb marked this pull request as ready for review September 9, 2022 15:49
@@ -691,7 +779,7 @@ mod tests {
agg,
bucket,
'1 day'::interval,
LAG(agg) OVER (ORDER BY bucket),
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the trailing whitespace here for consistency with the similar SQL for getting the integrals.

extension/src/time_weighted_average.rs Outdated Show resolved Hide resolved
extension/src/time_weighted_average.rs Outdated Show resolved Hide resolved
extension/src/accessors.rs Outdated Show resolved Hide resolved
@syvb syvb force-pushed the smittyvb/integral branch 3 times, most recently from 59ec1aa to 2ce21ce Compare September 16, 2022 12:00
Copy link
Contributor

@rtwalker rtwalker 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 changes! Looks good to me though I'd wait for someone else to weigh in :)

Copy link
Contributor

@davidkohn88 davidkohn88 left a comment

Choose a reason for hiding this comment

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

LGTM!

@syvb
Copy link
Member Author

syvb commented Sep 20, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 20, 2022

@bors bors bot merged commit 7f6a0f9 into main Sep 20, 2022
@bors bors bot deleted the smittyvb/integral branch September 20, 2022 01:21
@rtwalker rtwalker mentioned this pull request Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants