fixing interpolation logic #514
Conversation
| ) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
Can we get a test for gauge_agg too? One of these days I need to get back to deduplicating these...
There was a problem hiding this comment.
Yes, copied the counter_agg test here. We should look at deduplicating this for the next release or the following one (probably as part of stabilizing gauge_agg).
| @@ -62,7 +62,7 @@ impl<'input> TimeWeightSummary<'input> { | |||
| _ => self.first | |||
| }; | |||
| let new_end = match next { | |||
| Some(next) if self.last.ts < interval_start + interval_len => { | |||
There was a problem hiding this comment.
The removed if should never have failed on valid input (and with the assert fixed is guaranteed to not fail).
| @@ -48,7 +48,7 @@ impl<'input> TimeWeightSummary<'input> { | |||
| prev: Option<TimeWeightSummary>, | |||
| next: Option<TimeWeightSummary>, | |||
| ) -> TimeWeightSummary<'static> { | |||
| assert!(interval_start <= self.first.ts && interval_start + interval_len >= self.last.ts); | |||
| assert!(interval_start <= self.first.ts && interval_start + interval_len > self.last.ts); | |||
There was a problem hiding this comment.
The test still passes without this change.
There was a problem hiding this comment.
This is just fixing the assert to reflect that the aggregate is exclusive on the upper bound. If this assert fails, then the caller is trying to call an interpolated function using an interval that's smaller than their bucketing interval. It might still be useful to add a test that we handle invalid input correctly, but pg_test still has trouble with failure tests.
WireBaron
force-pushed
the
br/interpolated_delta_fix
branch
from
0b450d5
to
7f8996d
Compare
|
bors r+ |
|
Build succeeded: |
This change avoids creating a new start point for an interpolated aggregate if the existing start point is at the interval boundary.
Fixes #499