[ES|QL] Add variations of standard deviation functions #129129
Draft
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
limotova
force-pushed
the
add-stddev-population
branch
from
0630a50 to
9a5b800
Compare
limotova
force-pushed
the
add-stddev-population
branch
from
2690c24 to
08feb83
Compare
limotova
force-pushed
the
add-stddev-population
branch
from
08feb83 to
58d858a
Compare
limotova
force-pushed
the
add-stddev-population
branch
from
58d858a to
6450343
Compare
limotova
changed the title
limotova
changed the title
limotova
force-pushed
the
add-stddev-population
branch
from
6450343 to
4b3866a
Compare
limotova
force-pushed
the
add-stddev-population
branch
from
4b3866a to
2b7feb0
Compare
limotova
force-pushed
the
add-stddev-population
branch
from
2b7feb0 to
356dec3
Compare
|
🔍 Preview links for changed docs: 🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
dnhatn
reviewed
Jun 26, 2025
| def(Top.class, tri(Top::new), "top"), | ||
| def(Values.class, uni(Values::new), "values"), | ||
| def(VariancePopulation.class, uni(VariancePopulation::new), "variance_population"), | ||
| def(VarianceSample.class, uni(VarianceSample::new), "variance_sample"), |
| def(Sum.class, uni(Sum::new), "sum"), | ||
| def(Top.class, tri(Top::new), "top"), | ||
| def(Values.class, uni(Values::new), "values"), | ||
| def(VariancePopulation.class, uni(VariancePopulation::new), "variance_population"), |
There was a problem hiding this comment.
Should this be population_variance?
| import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT; | ||
| import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType; | ||
|
|
||
| public class $Variation$ extends AggregateFunction implements ToAggregator { |
| public final AggregatorFunctionSupplier supplier() { | ||
| DataType type = field().dataType(); | ||
| if (type == DataType.LONG) { | ||
| return new $Variation$LongAggregatorFunctionSupplier(); |
There was a problem hiding this comment.
Can we add a new parameter for the variance type to the aggregator (see CountDistinctIntAggregator) so we don't have to generate similar classes multiple times? This approach is also fine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
The bulk of this PR is generated code
For ease of reading, the first 4 commits are split into the following components:
STD_DEV_SAMPLE,VARIANCE_POPULATION, andVARIANCE_SAMPLE(all new functions)STD_DEV(the previously existing function)Currently ES|QL only has a population standard deviation aggregation
STD_DEVavailable.This PR adds in
STD_DEV_SAMPLE,VARIANCE_POPULATION, andVARIANCE_SAMPLEfunctions by expanding the originalX-StdDevAggregator.java.sttemplate to output each of the 4 variations, as well as introducing aX-StdDev.java.stto generate the AggregateFunction classes for all 4.