Skip to content

Commit 95bfac9

Browse files
committed
SQL: Translate MIN/MAX on keyword fields as FIRST/LAST (#41247)
Although the translation rule was implemented in the `Optimizer`, the rule was not added in the list of rules to be executed. Relates to #41195 Follows #37936 (cherry picked from commit f426a33)
1 parent 6f7751f commit 95bfac9

File tree

3 files changed

+45
-0
lines changed

3 files changed

+45
-0
lines changed

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,26 @@ SELECT COUNT(ALL last_name)=COUNT(ALL first_name) AS areEqual, COUNT(ALL first_n
414414
false |90 |100
415415
;
416416

417+
topHitsAsMinAndMax
418+
schema::min:s|max:s|first:s|last:s
419+
SELECT MIN(first_name) as min, MAX(first_name) as max, FIRST(first_name) as first, LAST(first_name) as last FROM test_emp;
420+
421+
min | max | first | last
422+
---------------+---------------+--------------+----------
423+
Alejandro | Zvonko | Alejandro | Zvonko
424+
;
425+
426+
topHitsAsMinAndMaxAndGroupBy
427+
schema::gender:s|min:s|max:s|first:s|last:s
428+
SELECT gender, MIN(first_name) as min, MAX(first_name) as max, FIRST(first_name) as first, LAST(first_name) as last FROM test_emp GROUP BY gender ORDER BY gender;
429+
430+
gender | min | max | first | last
431+
---------------+---------------+--------------+---------------+----------
432+
null | Berni | Patricio | Berni | Patricio
433+
F | Alejandro | Xinglin | Alejandro | Xinglin
434+
M | Amabile | Zvonko | Amabile | Zvonko
435+
;
436+
417437
topHitsWithOneArgAndGroupBy
418438
schema::gender:s|first:s|last:s
419439
SELECT gender, FIRST(first_name) as first, LAST(first_name) as last FROM test_emp GROUP BY gender ORDER BY gender;

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ protected Iterable<RuleExecutor<LogicalPlan>.Batch> batches() {
149149

150150
Batch aggregate = new Batch("Aggregation Rewrite",
151151
//new ReplaceDuplicateAggsWithReferences(),
152+
new ReplaceMinMaxWithTopHits(),
152153
new ReplaceAggsWithMatrixStats(),
153154
new ReplaceAggsWithExtendedStats(),
154155
new ReplaceAggsWithStats(),

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,18 @@ public void testTopHitsAggregationWithOneArg() {
763763
"\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," +
764764
"\"sort\":[{\"keyword\":{\"order\":\"asc\",\"missing\":\"_last\",\"unmapped_type\":\"keyword\"}}]}}}}}"));
765765
}
766+
{
767+
PhysicalPlan p = optimizeAndPlan("SELECT MIN(keyword) FROM test");
768+
assertEquals(EsQueryExec.class, p.getClass());
769+
EsQueryExec eqe = (EsQueryExec) p;
770+
assertEquals(1, eqe.output().size());
771+
assertEquals("MIN(keyword)", eqe.output().get(0).qualifiedName());
772+
assertEquals(DataType.KEYWORD, eqe.output().get(0).dataType());
773+
assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""),
774+
endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," +
775+
"\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," +
776+
"\"sort\":[{\"keyword\":{\"order\":\"asc\",\"missing\":\"_last\",\"unmapped_type\":\"keyword\"}}]}}}}}"));
777+
}
766778
{
767779
PhysicalPlan p = optimizeAndPlan("SELECT LAST(date) FROM test");
768780
assertEquals(EsQueryExec.class, p.getClass());
@@ -775,6 +787,18 @@ public void testTopHitsAggregationWithOneArg() {
775787
"\"explain\":false,\"docvalue_fields\":[{\"field\":\"date\",\"format\":\"epoch_millis\"}]," +
776788
"\"sort\":[{\"date\":{\"order\":\"desc\",\"missing\":\"_last\",\"unmapped_type\":\"date\"}}]}}}}}"));
777789
}
790+
{
791+
PhysicalPlan p = optimizeAndPlan("SELECT MAX(keyword) FROM test");
792+
assertEquals(EsQueryExec.class, p.getClass());
793+
EsQueryExec eqe = (EsQueryExec) p;
794+
assertEquals(1, eqe.output().size());
795+
assertEquals("MAX(keyword)", eqe.output().get(0).qualifiedName());
796+
assertEquals(DataType.KEYWORD, eqe.output().get(0).dataType());
797+
assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""),
798+
endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," +
799+
"\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," +
800+
"\"sort\":[{\"keyword\":{\"order\":\"desc\",\"missing\":\"_last\",\"unmapped_type\":\"keyword\"}}]}}}}}"));
801+
}
778802
}
779803

780804
public void testTopHitsAggregationWithTwoArgs() {

0 commit comments

Comments
 (0)