SQL: ORDER BY column and aggregate orders only by aggregate #50355
Assignees
Labels
Comments
|
Pinging @elastic/es-search (:Search/SQL) |
matriv
added a commit
to matriv/elasticsearch
that referenced
this issue
Feb 4, 2020
Previously, in the in-memory sorting module `LocalAggregationSorterListener` only the aggregate functions where used (grabbed by the `sortingColumns`). As a consequence, if the ORDER BY was also using columns of the GROUP BY clause, (especially in the case of higher priority - before the aggregate functions) wrong results were produced. E.g.: ``` SELECT gender, MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY gender, max ``` Add all columns of the ORDER BY to the `sortingColumns` so that the `LocalAggregationSorterListener` can use the correct comparators in the underlying PriorityQueue used to implement the in-memory sorting. Fixes: elastic#50355
matriv
added a commit
that referenced
this issue
Feb 12, 2020
Previously, in the in-memory sorting module `LocalAggregationSorterListener` only the aggregate functions where used (grabbed by the `sortingColumns`). As a consequence, if the ORDER BY was also using columns of the GROUP BY clause, (especially in the case of higher priority - before the aggregate functions) wrong results were produced. E.g.: ``` SELECT gender, MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY gender, max ``` Add all columns of the ORDER BY to the `sortingColumns` so that the `LocalAggregationSorterListener` can use the correct comparators in the underlying PriorityQueue used to implement the in-memory sorting. Fixes: #50355
matriv
added a commit
that referenced
this issue
Feb 12, 2020
Previously, in the in-memory sorting module `LocalAggregationSorterListener` only the aggregate functions where used (grabbed by the `sortingColumns`). As a consequence, if the ORDER BY was also using columns of the GROUP BY clause, (especially in the case of higher priority - before the aggregate functions) wrong results were produced. E.g.: ``` SELECT gender, MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY gender, max ``` Add all columns of the ORDER BY to the `sortingColumns` so that the `LocalAggregationSorterListener` can use the correct comparators in the underlying PriorityQueue used to implement the in-memory sorting. Fixes: #50355 (cherry picked from commit be680af)
matriv
added a commit
that referenced
this issue
Feb 12, 2020
Previously, in the in-memory sorting module `LocalAggregationSorterListener` only the aggregate functions where used (grabbed by the `sortingColumns`). As a consequence, if the ORDER BY was also using columns of the GROUP BY clause, (especially in the case of higher priority - before the aggregate functions) wrong results were produced. E.g.: ``` SELECT gender, MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY gender, max ``` Add all columns of the ORDER BY to the `sortingColumns` so that the `LocalAggregationSorterListener` can use the correct comparators in the underlying PriorityQueue used to implement the in-memory sorting. Fixes: #50355 (cherry picked from commit be680af)
matriv
added a commit
that referenced
this issue
Feb 12, 2020
Previously, in the in-memory sorting module `LocalAggregationSorterListener` only the aggregate functions where used (grabbed by the `sortingColumns`). As a consequence, if the ORDER BY was also using columns of the GROUP BY clause, (especially in the case of higher priority - before the aggregate functions) wrong results were produced. E.g.: ``` SELECT gender, MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY gender, max ``` Add all columns of the ORDER BY to the `sortingColumns` so that the `LocalAggregationSorterListener` can use the correct comparators in the underlying PriorityQueue used to implement the in-memory sorting. Fixes: #50355 (cherry picked from commit be680af)
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Query to test on SQL's test data:
SELECT gender, MIN(salary) AS min, COUNT(*) AS c, MAX(salary) FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender ASC, MAX(salary) DESCwhich gives:The result seems to indicate that the ordering is done ONLY on the aggregate. Changing the directions of ORDER BYs -
SELECT gender, MIN(salary) AS min, COUNT(*) AS c, MAX(salary) FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender DESC, MAX(salary) ASCseems to support the above assumption:Our tests DO cover this scenario but only partially.
In
agg-ordering.sql-specwe havewhich runs ok, but only because ordering by
genderand ordering byMAX(salary)offer the same results forASCordering. A better test should also returnMAX(salary)as a projection and should mixASCandDESCas directions ofORDER BY.The text was updated successfully, but these errors were encountered: