Skip to content

Commit a8f18a4

Browse files
committed
SQL: Fix LIMIT bug in agg sorting (#41258)
When specifying a limit over an agg sorting, the limit will be pushed down to the grouping which affects the custom sorting. This commit fixes that and restricts the limit only to sorting. Fix #40984 (cherry picked from commit da37265)
1 parent 38bb047 commit a8f18a4

File tree

2 files changed

+26
-1
lines changed

2 files changed

+26
-1
lines changed

x-pack/plugin/sql/qa/src/main/resources/agg-ordering.sql-spec

+24
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,24 @@ SELECT MAX(salary) AS max, MIN(salary) AS min FROM test_emp HAVING MIN(salary) >
2929
aggWithoutAlias
3030
SELECT MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY MAX(salary);
3131

32+
aggWithoutAliasWithLimit
33+
SELECT MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY MAX(salary) LIMIT 3;
34+
35+
aggWithoutAliasWithLimitDesc
36+
SELECT MAX(salary) AS max FROM test_emp GROUP BY gender ORDER BY MAX(salary) DESC LIMIT 3;
37+
3238
aggWithAlias
3339
SELECT MAX(salary) AS m FROM test_emp GROUP BY gender ORDER BY m;
3440

41+
aggOrderByCountWithLimit
42+
SELECT MAX(salary) AS max, COUNT(*) AS c FROM test_emp GROUP BY gender ORDER BY c LIMIT 3;
43+
44+
aggOrderByCountWithLimitDescAndGrouping
45+
SELECT gender, COUNT(*) AS c FROM test_emp GROUP BY gender ORDER BY c DESC LIMIT 5;
46+
47+
aggOrderByCountWithLimitDesc
48+
SELECT MAX(salary) AS max, COUNT(*) AS c FROM test_emp GROUP BY gender ORDER BY c DESC LIMIT 3;
49+
3550
multipleAggsThatGetRewrittenWithoutAlias
3651
SELECT MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY gender ORDER BY MAX(salary);
3752

@@ -56,12 +71,21 @@ SELECT MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c
5671
aggNotSpecifiedInTheAggregateAndGroupWithHaving
5772
SELECT gender, MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY MAX(salary), gender;
5873

74+
aggNotSpecifiedInTheAggregateAndGroupWithHavingWithLimit
75+
SELECT gender, MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY MAX(salary), c LIMIT 5;
76+
77+
aggNotSpecifiedInTheAggregateAndGroupWithHavingWithLimitAndDirection
78+
SELECT gender, MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY MAX(salary) ASC, c DESC LIMIT 5;
79+
5980
groupAndAggNotSpecifiedInTheAggregateWithHaving
6081
SELECT gender, MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender, MAX(salary);
6182

6283
multipleAggsThatGetRewrittenWithAliasOnAMediumGroupBy
6384
SELECT languages, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY languages ORDER BY max;
6485

86+
multipleAggsThatGetRewrittenWithAliasOnAMediumGroupByWithLimit
87+
SELECT languages, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY languages ORDER BY max DESC LIMIT 5;
88+
6589
multipleAggsThatGetRewrittenWithAliasOnALargeGroupBy
6690
SELECT emp_no, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY emp_no ORDER BY max;
6791

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public static SearchSourceBuilder sourceBuilder(QueryContainer container, QueryB
8080
if (source.size() == -1) {
8181
source.size(sz);
8282
}
83-
if (aggBuilder instanceof CompositeAggregationBuilder) {
83+
// limit the composite aggs only for non-local sorting
84+
if (aggBuilder instanceof CompositeAggregationBuilder && container.sortingColumns().isEmpty()) {
8485
((CompositeAggregationBuilder) aggBuilder).size(sz);
8586
}
8687
}

0 commit comments

Comments
 (0)