ESQL: Fix Max doubles bug with negatives and add tests for Max and Min #110586
Conversation
ivancea
added
>bug
Team:Analytics
|
Documentation preview: |
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Pinging @elastic/kibana-esql (ES|QL-ui) |
|
Hi @ivancea, I've created a changelog YAML for you. |
| @@ -166,7 +168,9 @@ private void aggregateWithIntermediates(Expression expression) { | |||
|
|
|||
| Page inputPage = rows(testCase.getMultiRowFields()); | |||
| try { | |||
| aggregator.processPage(inputPage); | |||
| if (inputPage.getPositionCount() > 0) { | |||
There was a problem hiding this comment.
This is fine for now, but I wonder if we should make rows make a List<Page> and we dump them all in. So if it spits out no pages that's cool. At some point we're going to have to signal "empty" with no results, but that could live in rows.
There was a problem hiding this comment.
Changed it already. Maybe check it; I'm creating pages of 1-10 elements. Went with the small values as some hand-made tests may have just a few elements. But it should still work either way, so whatever
| rowsCount | ||
| ) | ||
| ) { | ||
| int pageSize = randomIntBetween(1, 10); |
There was a problem hiding this comment.
10,000 is a pretty normal size in production. Maybe randomBoolean() ? between(1, 100) : Math.toIntExact(BlockFactory.DEFAULT_MAX_BLOCK_PRIMITIVE_ARRAY_SIZE.getBytes()). Half the time we'll have degenerately small data and half the time it'll probably just be one page.
ivancea
added
the
auto-merge-without-approval
# Conflicts: # docs/reference/esql/functions/aggregation-functions.asciidoc
|
backport incoming |
Partial backport from #110586 Just the Max fix and an extra test for it.
#110586) `MAX()` currently doesn't work with doubles smaller than `Double.MIN_VALUE` (Note that `Double.MIN_VALUE` returns the smallest non-zero positive, not the smallest double). This PR adds tests for Max and Min, and fixes the bug (Detected by the tests). Also, as the tests now generate the docs, replaced the old docs with the generated ones, and updated the Max&Min examples.
#110586) `MAX()` currently doesn't work with doubles smaller than `Double.MIN_VALUE` (Note that `Double.MIN_VALUE` returns the smallest non-zero positive, not the smallest double). This PR adds tests for Max and Min, and fixes the bug (Detected by the tests). Also, as the tests now generate the docs, replaced the old docs with the generated ones, and updated the Max&Min examples.
MAX()currently doesn't work with doubles smaller thanDouble.MIN_VALUE(Note thatDouble.MIN_VALUEreturns the smallest non-zero positive, not the smallest double).This PR adds tests for Max and Min, and fixes the bug (Detected by the tests).
Also, as the tests now generate the docs, replaced the old docs with the generated ones, and updated the Max&Min examples.