-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ESQL: Prevent overflow on SUM using multiple aggregators #116170
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
Closed
Closed
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
7a872cf
Remove unused empty constructor
ivancea 87db6ac
Added fallible single states
ivancea 17e9ea2
Added fallible array state
ivancea 318d5cd
Added custom warning object to aggregator function
ivancea 4937082
Completed single aggregator implementation
ivancea 7935330
Updated SumTests and format
ivancea 35f7002
Update docs/changelog/111639.yaml
ivancea 0fb3040
Fix tests compilation
ivancea 0d33140
Add warn exceptions in grouping aggregator
ivancea 523a583
Update docs/changelog/111639.yaml
ivancea 1b09dd3
Early returns on non-grouping, and todos for missing catch
ivancea b1fdc25
Add try-catch in intermediate state
ivancea 16b1cf8
Release failed array, and fixed warnings on grouping tests
ivancea d6d44c7
Fixed benchmark compilation
ivancea 31cb9d1
Merge branch 'main' into sum-warns
ivancea 10ae7b2
Extend from AbstractArrayState, and extract annotations method
ivancea e95cc8d
Removed obsolete test
ivancea 55ac3b8
Merge branch 'main' into sum-warns
ivancea 5530b3a
Merge branch 'main' into sum-warns
ivancea c76bb62
Merge branch 'main' into sum-warns
ivancea 753af10
Test warnings for long overflow
ivancea 7b2e1c7
Merge branch 'main' into sum-warns
ivancea 678d183
Merge branch 'main' into sum-warns
ivancea ce4a448
Merge branch 'main' into sum-warns
ivancea 086ee5a
Merge branch 'main' into sum-warns
ivancea d71eea2
Added CSV test failing in mixed cluster
ivancea 5dd8762
Merge branch 'main' into sum-warns
ivancea c4a0aef
Merge branch 'main' into sum-warns
ivancea ce9d7c3
Merge branch 'main' into sum-warns
ivancea 66ab25f
Merge branch 'main' into sum-warns-cluster-feature
ivancea 919403c
Added features to Configuration, and use it in aggregate functions
ivancea 5d026c2
Updated tests with config, and fixed serialization and AggregateMappe…
ivancea d207d68
Added old aggregator, AggregatorMapper change remaining
ivancea fd69d05
Added aggregator extra tests, and AggregateMapper updates to allow fo…
ivancea 5b525f4
Format
ivancea fa3d7a4
Merge branch 'main' into sum-warns
ivancea 0a1e99f
Merge branch 'sum-warns' into sum-warns-cluster-feature
ivancea bde36dd
Added required capability to CSV test on overflows
ivancea 0978eeb
Update docs/changelog/116170.yaml
ivancea 423549d
Remove old branch changelog entry
ivancea fb98bec
Add "final" again to AggregateFunction#writeTo
ivancea abcd246
Fixed serialization of functions with configuration
ivancea b9d66b4
Format
ivancea 134a0d4
Fixed PhysicalOptimizerTests not using same config for everything
ivancea 20d9adf
Format
ivancea cde0ed7
Merge branch 'main' into sum-warns-cluster-feature
ivancea File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
pr: 116170 | ||
summary: "ESQL: Prevent overflow on SUM using multiple aggregators" | ||
area: ES|QL | ||
type: bug | ||
issues: | ||
- 110443 |
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
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
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
181 changes: 181 additions & 0 deletions
181
...generated/org/elasticsearch/compute/aggregation/OverflowingSumLongAggregatorFunction.java
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
39 changes: 39 additions & 0 deletions
39
...d/org/elasticsearch/compute/aggregation/OverflowingSumLongAggregatorFunctionSupplier.java
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.
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.
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.
This was a bug in the PR implementing warnExceptions on aggs, which wasn't being used anywhere yet