-
Notifications
You must be signed in to change notification settings - Fork 53
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
Adding asof joins for timevector #635
Conversation
.map(|points| (points.ts.into(), points.val)) | ||
.peekable(); | ||
let into = into.into_iter().map(|points| (points.ts, points.val)); | ||
let (mut from_time, mut from_val) = from.next().unwrap(); |
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.
The default error message from .unwrap()
isn't particularly user friendly:
let (mut from_time, mut from_val) = from.next().unwrap(); | |
let (mut from_time, mut from_val) = from.next().expect("Must have at least one point in joining column"); |
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.
Good point, however it's also going to have potentially ugly behavior if the into vector is empty, so I added an assert that neither is empty to the start of this function.
77c8a42
to
7d4dafc
Compare
> { | ||
assert!( | ||
from.num_points > 0 && into.num_points > 0, | ||
"both timevectors must be populated for an asof join" |
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.
Can we test these two errors?
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.
Added...it's actually surprisingly difficult to create an empty timevector.
7d4dafc
to
dd0a0ee
Compare
@@ -7,11 +7,14 @@ This changelog should be updated as part of a PR if the work is worth noting (mo | |||
## Next Release (Date TBD) | |||
|
|||
#### New experimental features | |||
- [#615](https://github.com/timescale/timescaledb-toolkit/pull/614): Heatbeat aggregate | |||
- [#615](https://github.com/timescale/timescaledb-toolkit/pull/615): Heatbeat aggregate |
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.
Hehe we caught the same thing! Also "heatbeat" though, and also there are some rendering problems without blank lines. I sent a pull request for these.
Users can use the new `heartbeat_agg(timestamp, start_time, agg_interval, heartbeat_interval)` to track the liveness of a system in the range (`start_time`, `start_time` + `agg_interval`). Each timestamp seen in that range is assumed to indicate system liveness for the following `heartbeat_interval`. | ||
Once constructed, users can query heartbeat aggregates for `uptime` and `downtime`, as well as query for `live_ranges` or `dead_ranges`. Users can also check for `live_at(timestamp)`. | ||
Heartbeat aggregates can also interpolated to better see behavior around the boundaries of the individual aggregates. | ||
|
||
- [#635](https://github.com/timescale/timescaledb-toolkit/pull/635): AsOf joins for timevectors | ||
This allows users to join two timevectors with the following semantics `timevectorA -> asof(timevectorB)`. This will return records with the LOCF value from timevectorA at the timestamps from timevectorB. Specifically the returned records contain, for each value in timevectorB, {the LOCF value from timevectorA, the value from timevectorB, the timestamp from timevectorB}. |
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.
Without a blank line between lines 15 and 16 here, Github squishes it all into one paragraph. The blank does not disrupt the list.
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.
Fixed this and merged your fix for the above. Will merge once I verify everything looks good.
This change adds a new function `join_asof` which takes two timevectors and outputs the values of the first at the times given in the second (as well as the values in the second).
dd0a0ee
to
944c5de
Compare
bors r+ |
Build succeeded:
|
This change adds a new function
join_asof
which takes two timevectors and outputs the values of the first at the times given in the second (as well as the values in the second).Fixes #633