From 45ec73db302dc35a9a6178d38782dbf891f638de Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Wed, 12 Feb 2020 09:37:36 +0100 Subject: [PATCH] SQL: Fix ORDER BY on aggregates and GROUPed BY fields (#51894) 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 be680af11c823292c2d115bff01658f7b75abd76) --- .../src/main/resources/agg-ordering.csv-spec | 48 +++++++++- .../src/main/resources/agg-ordering.sql-spec | 27 +++++- .../xpack/sql/execution/search/Querier.java | 47 ++++++---- .../sql/execution/search/SourceGenerator.java | 2 +- .../xpack/sql/planner/QueryFolder.java | 32 ++++--- .../querydsl/container/QueryContainer.java | 91 ++++++++++--------- .../sql/execution/search/QuerierTests.java | 66 +++++++++++++- .../search/SourceGeneratorTests.java | 13 +-- 8 files changed, 242 insertions(+), 84 deletions(-) diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.csv-spec index ce96c34344ab8..070abdb68b3fb 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.csv-spec @@ -23,4 +23,50 @@ g:s | gender:s | s3:i | SUM(salary):i | s5:i M |M |2671054|2671054 |2671054 F |F |1666196|1666196 |1666196 null |null |487605 |487605 |487605 -; \ No newline at end of file +; + +histogramDateTimeWithCountAndOrder_1 +schema::h:ts|c:l +SELECT HISTOGRAM(birth_date, INTERVAL 1 YEAR) AS h, COUNT(*) as c FROM test_emp GROUP BY h ORDER BY h DESC, c ASC; + + h | c +------------------------+--------------- +1965-01-01T00:00:00.000Z|1 +1964-01-01T00:00:00.000Z|4 +1963-01-01T00:00:00.000Z|7 +1962-01-01T00:00:00.000Z|6 +1961-01-01T00:00:00.000Z|8 +1960-01-01T00:00:00.000Z|8 +1959-01-01T00:00:00.000Z|9 +1958-01-01T00:00:00.000Z|7 +1957-01-01T00:00:00.000Z|4 +1956-01-01T00:00:00.000Z|5 +1955-01-01T00:00:00.000Z|4 +1954-01-01T00:00:00.000Z|8 +1953-01-01T00:00:00.000Z|11 +1952-01-01T00:00:00.000Z|8 +null |10 +; + +histogramDateTimeWithCountAndOrder_2 +schema::h:ts|c:l +SELECT HISTOGRAM(birth_date, INTERVAL 1 YEAR) AS h, COUNT(*) as c FROM test_emp GROUP BY h ORDER BY c DESC, h ASC; + + h | c +------------------------+--------------- +1953-01-01T00:00:00.000Z|11 +null |10 +1959-01-01T00:00:00.000Z|9 +1952-01-01T00:00:00.000Z|8 +1954-01-01T00:00:00.000Z|8 +1960-01-01T00:00:00.000Z|8 +1961-01-01T00:00:00.000Z|8 +1958-01-01T00:00:00.000Z|7 +1963-01-01T00:00:00.000Z|7 +1962-01-01T00:00:00.000Z|6 +1956-01-01T00:00:00.000Z|5 +1955-01-01T00:00:00.000Z|4 +1957-01-01T00:00:00.000Z|4 +1964-01-01T00:00:00.000Z|4 +1965-01-01T00:00:00.000Z|1 +; diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.sql-spec b/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.sql-spec index 9a193d76b3166..3a2f63bc47a37 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.sql-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg-ordering.sql-spec @@ -78,7 +78,7 @@ aggNotSpecifiedInTheAggregateAndGroupWithHavingWithLimitAndDirection 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; groupAndAggNotSpecifiedInTheAggregateWithHaving -SELECT gender, MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender, MAX(salary); +SELECT gender, MIN(salary) AS min, COUNT(*) AS c FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender NULLS FIRST, MAX(salary); multipleAggsThatGetRewrittenWithAliasOnAMediumGroupBy SELECT languages, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY languages ORDER BY max; @@ -134,5 +134,26 @@ SELECT gender AS g, first_name AS f, last_name AS l FROM test_emp GROUP BY f, ge multipleGroupingsAndOrderingByGroups_8 SELECT gender AS g, first_name, last_name FROM test_emp GROUP BY g, last_name, first_name ORDER BY gender ASC, first_name DESC, last_name ASC; -multipleGroupingsAndOrderingByGroupsWithFunctions -SELECT first_name f, last_name l, gender g, CONCAT(first_name, last_name) c FROM test_emp GROUP BY gender, l, f, c ORDER BY gender, c DESC, first_name, last_name ASC; +multipleGroupingsAndOrderingByGroupsAndAggs_1 +SELECT gender, MIN(salary) AS min, COUNT(*) AS c, MAX(salary) AS max FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender ASC NULLS FIRST, MAX(salary) DESC; + +multipleGroupingsAndOrderingByGroupsAndAggs_2 +SELECT gender, MIN(salary) AS min, COUNT(*) AS c, MAX(salary) AS max FROM test_emp GROUP BY gender HAVING c > 1 ORDER BY gender DESC NULLS LAST, MAX(salary) ASC; + +multipleGroupingsAndOrderingByGroupsWithFunctions_1 +SELECT first_name f, last_name l, gender g, CONCAT(first_name, last_name) c FROM test_emp GROUP BY gender, l, f, c ORDER BY gender NULLS FIRST, c DESC, first_name, last_name ASC; + +multipleGroupingsAndOrderingByGroupsWithFunctions_2 +SELECT first_name f, last_name l, gender g, CONCAT(first_name, last_name) c FROM test_emp GROUP BY gender, l, f, c ORDER BY c DESC, gender DESC NULLS LAST, first_name, last_name ASC; + +multipleGroupingsAndOrderingByGroupsAndAggregatesWithFunctions_1 +SELECT CONCAT('foo', gender) g, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY g ORDER BY 1 NULLS FIRST, 2, 3; + +multipleGroupingsAndOrderingByGroupsAndAggregatesWithFunctions_2 +SELECT CONCAT('foo', gender) g, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY g ORDER BY 1 DESC NULLS LAST, 2, 3; + +multipleGroupingsAndOrderingByGroupsAndAggregatesWithFunctions_3 +SELECT CONCAT('foo', gender) g, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY g ORDER BY 2, 1 NULLS FIRST, 3; + +multipleGroupingsAndOrderingByGroupsAndAggregatesWithFunctions_4 +SELECT CONCAT('foo', gender) g, MAX(salary) AS max, MIN(salary) AS min FROM test_emp GROUP BY g ORDER BY 3 DESC, 1 NULLS FIRST, 2; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java index 1a4a3282800ab..2b53fc23f121d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java @@ -578,36 +578,51 @@ static class AggSortingQueue extends PriorityQueue, Integer>> { this.sortingColumns = sortingColumns; } - // compare row based on the received attribute sort - // if a sort item is not in the list, it is assumed the sorting happened in ES - // and the results are left as is (by using the row ordering), otherwise it is sorted based on the given criteria. - // - // Take for example ORDER BY a, x, b, y - // a, b - are sorted in ES - // x, y - need to be sorted client-side - // sorting on x kicks in, only if the values for a are equal. - + /** + * Compare row based on the received attribute sort + * + * + * Take for example ORDER BY a, x, b, y + * a, b - are sorted in ES + * x, y - need to be sorted client-side + * sorting on x kicks in only if the values for a are equal. + * sorting on y kicks in only if the values for a, x and b are all equal + * + */ // thanks to @jpountz for the row ordering idea as a way to preserve ordering @SuppressWarnings("unchecked") @Override protected boolean lessThan(Tuple, Integer> l, Tuple, Integer> r) { for (Tuple tuple : sortingColumns) { - int i = tuple.v1().intValue(); + int columnIdx = tuple.v1().intValue(); Comparator comparator = tuple.v2(); - Object vl = l.v1().get(i); - Object vr = r.v1().get(i); + // Get the values for left and right rows at the current column index + Object vl = l.v1().get(columnIdx); + Object vr = r.v1().get(columnIdx); if (comparator != null) { int result = comparator.compare(vl, vr); - // if things are equals, move to the next comparator + // if things are not equal: return the comparison result, + // otherwise: move to the next comparator to solve the tie. if (result != 0) { return result > 0; } } - // no comparator means the existing order needs to be preserved + // no comparator means the rows are pre-ordered by ES for the column at + // the current index and the existing order needs to be preserved else { - // check the values - if they are equal move to the next comparator - // otherwise return the row order + // check the values - if they are not equal return the row order + // otherwise: move to the next comparator to solve the tie. if (Objects.equals(vl, vr) == false) { return l.v2().compareTo(r.v2()) > 0; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java index ab312fd984685..41ae12141fc3b 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java @@ -105,7 +105,7 @@ private static void sorting(QueryContainer container, SearchSourceBuilder source source.sort("_doc"); return; } - for (Sort sortable : container.sort()) { + for (Sort sortable : container.sort().values()) { SortBuilder sortBuilder = null; if (sortable instanceof AttributeSort) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java index 807446e6eaf51..715f9e0a316a2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java @@ -22,6 +22,7 @@ import org.elasticsearch.xpack.sql.expression.function.Functions; import org.elasticsearch.xpack.sql.expression.function.ScoreAttribute; import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction; +import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunctionAttribute; import org.elasticsearch.xpack.sql.expression.function.aggregate.CompoundNumericAggregate; import org.elasticsearch.xpack.sql.expression.function.aggregate.Count; import org.elasticsearch.xpack.sql.expression.function.aggregate.InnerAggregate; @@ -461,20 +462,27 @@ protected PhysicalPlan rule(OrderExec plan) { GroupByKey group = qContainer.findGroupForAgg(attr); // TODO: might need to validate whether the target field or group actually exist - if (group != null && group != Aggs.IMPLICIT_GROUP_KEY) { + if (group!=null && group!=Aggs.IMPLICIT_GROUP_KEY) { qContainer = qContainer.updateGroup(group.with(direction)); } - else { - // scalar functions typically require script ordering - if (attr instanceof ScalarFunctionAttribute) { - // nope, use scripted sorting - qContainer = qContainer.addSort( - new ScriptSort(((ScalarFunctionAttribute) attr).script(), direction, missing)); - } else if (attr instanceof ScoreAttribute) { - qContainer = qContainer.addSort(new ScoreSort(direction, missing)); - } else { - qContainer = qContainer.addSort(new AttributeSort(attr, direction, missing)); - } + + // scalar functions typically require script ordering + if (attr instanceof ScalarFunctionAttribute) { + ScalarFunctionAttribute sf = (ScalarFunctionAttribute) attr; + // nope, use scripted sorting + qContainer = qContainer.addSort(sf.id(), new ScriptSort(sf.script(), direction, missing)); + } + // score + else if (attr instanceof ScoreAttribute) { + qContainer = qContainer.addSort(attr.id(), new ScoreSort(direction, missing)); + } + // agg function + else if (attr instanceof AggregateFunctionAttribute) { + AggregateFunctionAttribute afa = (AggregateFunctionAttribute) attr; + qContainer = qContainer.addSort(afa.innerId(), new AttributeSort(attr, direction, missing)); + // field, histogram + } else { + qContainer = qContainer.addSort(attr.id(), new AttributeSort(attr, direction, missing)); } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java index 2a0a14042aac9..e9050de39588c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java @@ -40,15 +40,12 @@ import java.util.Collections; import java.util.Comparator; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; -import static java.util.Collections.emptySet; import static java.util.Collections.singletonMap; import static org.elasticsearch.xpack.sql.util.CollectionUtils.combine; @@ -79,7 +76,7 @@ public class QueryContainer { // at scrolling, their inputs (leaves) get updated private final AttributeMap scalarFunctions; - private final Set sort; + private final Map sort; private final int limit; // computed @@ -96,7 +93,7 @@ public QueryContainer(Query query, Map aliases, Map pseudoFunctions, AttributeMap scalarFunctions, - Set sort, + Map sort, int limit) { this.query = query; this.aggs = aggs == null ? Aggs.EMPTY : aggs; @@ -104,7 +101,7 @@ public QueryContainer(Query query, this.aliases = aliases == null || aliases.isEmpty() ? Collections.emptyMap() : aliases; this.pseudoFunctions = pseudoFunctions == null || pseudoFunctions.isEmpty() ? emptyMap() : pseudoFunctions; this.scalarFunctions = scalarFunctions == null || scalarFunctions.isEmpty() ? AttributeMap.emptyAttributeMap() : scalarFunctions; - this.sort = sort == null || sort.isEmpty() ? emptySet() : sort; + this.sort = sort == null || sort.isEmpty() ? emptyMap() : sort; this.limit = limit; } @@ -118,46 +115,52 @@ public List> sortingColumns() { return emptyList(); } + for (Sort s : sort.values()) { + if (isAggregateSort(s)) { + customSort = Boolean.TRUE; + break; + } + } + + // If no custom sort is used break early + if (customSort == null) { + customSort = Boolean.FALSE; + return emptyList(); + } + List> sortingColumns = new ArrayList<>(sort.size()); + for (Map.Entry entry : sort.entrySet()) { + ExpressionId expressionId = entry.getKey(); + Sort s = entry.getValue(); - boolean aggSort = false; - for (Sort s : sort) { - Tuple tuple = new Tuple<>(Integer.valueOf(-1), null); - - if (s instanceof AttributeSort) { - AttributeSort as = (AttributeSort) s; - // find the relevant column of each aggregate function - if (as.attribute() instanceof AggregateFunctionAttribute) { - aggSort = true; - AggregateFunctionAttribute afa = (AggregateFunctionAttribute) as.attribute(); - afa = (AggregateFunctionAttribute) aliases.getOrDefault(afa.innerId(), afa); - int atIndex = -1; - for (int i = 0; i < fields.size(); i++) { - Tuple field = fields.get(i); - if (field.v2().equals(afa.innerId())) { - atIndex = i; - break; - } - } - - if (atIndex == -1) { - throw new SqlIllegalArgumentException("Cannot find backing column for ordering aggregation [{}]", afa.name()); - } - // assemble a comparator for it - Comparator comp = s.direction() == Sort.Direction.ASC ? Comparator.naturalOrder() : Comparator.reverseOrder(); - comp = s.missing() == Sort.Missing.FIRST ? Comparator.nullsFirst(comp) : Comparator.nullsLast(comp); - - tuple = new Tuple<>(Integer.valueOf(atIndex), comp); + int atIndex = -1; + for (int i = 0; i < fields.size(); i++) { + Tuple field = fields.get(i); + if (field.v2().equals(expressionId)) { + atIndex = i; + break; } } - sortingColumns.add(tuple); - } + if (atIndex == -1) { + throw new SqlIllegalArgumentException("Cannot find backing column for ordering aggregation [{}]", s); + } - if (customSort == null) { - customSort = Boolean.valueOf(aggSort); + // assemble a comparator for it, if it's not an AggregateSort + // then it's pre-sorted by ES so use null + Comparator comp = null; + if (isAggregateSort(s)) { + comp = s.direction() == Sort.Direction.ASC ? Comparator.naturalOrder() : Comparator.reverseOrder(); + comp = s.missing() == Sort.Missing.FIRST ? Comparator.nullsFirst(comp) : Comparator.nullsLast(comp); + } + + sortingColumns.add(new Tuple<>(Integer.valueOf(atIndex), comp)); } - return aggSort ? sortingColumns : emptyList(); + return sortingColumns; + } + + private boolean isAggregateSort(Sort s) { + return s instanceof AttributeSort && ((AttributeSort) s).attribute() instanceof AggregateFunctionAttribute; } /** @@ -212,7 +215,7 @@ public Map pseudoFunctions() { return pseudoFunctions; } - public Set sort() { + public Map sort() { return sort; } @@ -260,10 +263,10 @@ public QueryContainer withScalarProcessors(AttributeMap procs) { return new QueryContainer(query, aggs, fields, aliases, pseudoFunctions, procs, sort, limit); } - public QueryContainer addSort(Sort sortable) { - Set sort = new LinkedHashSet<>(this.sort); - sort.add(sortable); - return new QueryContainer(query, aggs, fields, aliases, pseudoFunctions, scalarFunctions, sort, limit); + public QueryContainer addSort(ExpressionId expressionId, Sort sortable) { + Map newSort = new LinkedHashMap<>(this.sort); + newSort.put(expressionId, sortable); + return new QueryContainer(query, aggs, fields, aliases, pseudoFunctions, scalarFunctions, newSort, limit); } private String aliasName(Attribute attr) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/QuerierTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/QuerierTests.java index a30a89addb816..2269aca7d8a9e 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/QuerierTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/QuerierTests.java @@ -66,6 +66,70 @@ public void testAggSorting_TwoFields() { } } + @SuppressWarnings("rawtypes") + public void testAggSorting_TwoFields_One_Presorted() { + List> tuples = new ArrayList<>(2); + tuples.add(new Tuple<>(0, null)); + tuples.add(new Tuple<>(1, Comparator.reverseOrder())); + Querier.AggSortingQueue queue = new AggSortingQueue(20, tuples); + + for (int i = 1; i <= 100; i++) { + queue.insertWithOverflow(new Tuple<>(Arrays.asList(i <= 5 ? null : 100 - i + 1, i), i)); + } + List> results = queue.asList(); + + assertEquals(20, results.size()); + for (int i = 0; i < 20; i++) { + assertEquals(i < 5 ? null : 100 - i, results.get(i).get(0)); + assertEquals(i < 5 ? 5 - i : i + 1, results.get(i).get(1)); + } + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + public void testAggSorting_FourFields() { + List comparators = Arrays.asList( + Comparator.naturalOrder(), + Comparator.naturalOrder(), + Comparator.reverseOrder(), + Comparator.naturalOrder() + ); + List> tuples = new ArrayList<>(4); + tuples.add(new Tuple<>(0, null)); + tuples.add(new Tuple<>(1, comparators.get(1))); + tuples.add(new Tuple<>(2, null)); + tuples.add(new Tuple<>(3, comparators.get(3))); + Querier.AggSortingQueue queue = new AggSortingQueue(35, tuples); + + List> expected = new ArrayList<>(128); + for (int i = 0; i < 128; i++) { + int col1 = i / 16; + int col2 = 15 - (i / 8); + int col3 = 32 - (i / 4); + int col4 = 127 - i; + + expected.add(Arrays.asList(col1, col2, col3, col4)); + queue.insertWithOverflow(new Tuple<>(Arrays.asList(col1, col2, col3, col4), i)); + } + + expected.sort((o1, o2) -> { + for (int i = 0; i < 4; i++) { + int result = comparators.get(i).compare(o1.get(i), o2.get(i)); + if (result != 0) { + return result; + } + } + return 0; + }); + List> results = queue.asList(); + + assertEquals(35, results.size()); + for (int i = 0; i < 35; i++) { + for (int j = 0; j < 4; j++) { + assertEquals(expected.get(i).get(j), results.get(i).get(j)); + } + } + } + @SuppressWarnings("rawtypes") public void testAggSorting_Randomized() { // Initialize comparators for fields (columns) @@ -76,7 +140,7 @@ public void testAggSorting_Randomized() { boolean order = randomBoolean(); ordering[j] = order; Comparator comp = order ? Comparator.naturalOrder() : Comparator.reverseOrder(); - tuples.add(new Tuple(j, comp)); + tuples.add(new Tuple<>(j, comp)); } // Insert random no of documents (rows) with random 0/1 values for each field diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java index 3c3010f16f11d..60b684afe78d7 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.expression.ExpressionId; import org.elasticsearch.xpack.sql.expression.FieldAttribute; import org.elasticsearch.xpack.sql.expression.function.Score; import org.elasticsearch.xpack.sql.querydsl.agg.AvgAgg; @@ -85,7 +86,7 @@ public void testSelectScoreForcesTrackingScore() { public void testSortScoreSpecified() { QueryContainer container = new QueryContainer() - .addSort(new ScoreSort(Direction.DESC, null)); + .addSort(new ExpressionId(), new ScoreSort(Direction.DESC, null)); SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10)); assertEquals(singletonList(scoreSort()), sourceBuilder.sorts()); } @@ -94,14 +95,14 @@ public void testSortFieldSpecified() { FieldSortBuilder sortField = fieldSort("test").unmappedType("keyword"); QueryContainer container = new QueryContainer() - .addSort(new AttributeSort(new FieldAttribute(Source.EMPTY, "test", new KeywordEsField("test")), Direction.ASC, - Missing.LAST)); + .addSort(new ExpressionId(), new AttributeSort(new FieldAttribute(Source.EMPTY, "test", new KeywordEsField("test")), + Direction.ASC, Missing.LAST)); SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10)); assertEquals(singletonList(sortField.order(SortOrder.ASC).missing("_last")), sourceBuilder.sorts()); container = new QueryContainer() - .addSort(new AttributeSort(new FieldAttribute(Source.EMPTY, "test", new KeywordEsField("test")), Direction.DESC, - Missing.FIRST)); + .addSort(new ExpressionId(), new AttributeSort(new FieldAttribute(Source.EMPTY, "test", new KeywordEsField("test")), + Direction.DESC, Missing.FIRST)); sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10)); assertEquals(singletonList(sortField.order(SortOrder.DESC).missing("_first")), sourceBuilder.sorts()); } @@ -118,4 +119,4 @@ public void testNoSortIfAgg() { SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10)); assertNull(sourceBuilder.sorts()); } -} \ No newline at end of file +}