Add evaluation context to time-series aggregators #126089
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dnhatn
force-pushed
the
generate-time-series-agg
branch
from
2d5dcdd to
ab04dfe
Compare
dnhatn
changed the title
dnhatn
force-pushed
the
generate-time-series-agg
branch
2 times, most recently
from
f0c3c5c to
80402d9
Compare
dnhatn
force-pushed
the
generate-time-series-agg
branch
from
80402d9 to
4a301dd
Compare
dnhatn
added
:Analytics/ES|QL
elasticsearchmachine
added
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
kkrik-es
reviewed
Apr 3, 2025
| private final ClassName implementation; | ||
| private final List<AggregatorImplementer.IntermediateStateDesc> intermediateState; | ||
| private final boolean includeTimestampVector; | ||
| private final boolean timseries; |
kkrik-es
reviewed
Apr 3, 2025
idegtiarenko
reviewed
Apr 4, 2025
| * A time-series aggregator might need more than just the DriverContext to evaluate, such as the range interval of each group. | ||
| */ | ||
| public class GroupingAggregatorEvaluationContext { | ||
| private final DriverContext driverContext; |
There was a problem hiding this comment.
NIT: maybe this could be a record
There was a problem hiding this comment.
Ah, I made this a class intentionally so that we can extend for TimeSeriesGroupingAggregatorEvaluationContext.
idegtiarenko
approved these changes
Apr 4, 2025
|
@kkrik-es @idegtiarenko Thanks for reviews. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rate aggregators need to access the start time and end time of each group to perform extrapolation in the final evaluation. This change replaces the DriverContext parameter in evaluateFinal with EvaluationContext, allowing the TimeSeriesAggregationOperator to pass an EvaluationContext with these time intervals. I took another approach where the extension is applied only for the time series aggregator, but it could be fragile with the pre-filter of aggregators.