From 6e09a10dc5557438aadbc60c8eb1860d60493f8c Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 16 Apr 2019 12:13:08 +0300 Subject: [PATCH 1/3] SQL: Translate MIN/MAX on keyword fields as FIRST/LAST 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 --- x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec | 11 +++++++++++ .../elasticsearch/xpack/sql/optimizer/Optimizer.java | 1 + 2 files changed, 12 insertions(+) diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec index b55c3f66eafd9..873a4e34aefbd 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec @@ -414,6 +414,17 @@ SELECT COUNT(ALL last_name)=COUNT(ALL first_name) AS areEqual, COUNT(ALL first_n false |90 |100 ; +topHitsAsMinAndMax +schema::gender:s|first:s|last:s +SELECT gender, MIN(first_name) as first, MAX(first_name) as last FROM test_emp GROUP BY gender ORDER BY gender; + + gender | first | last +---------------+---------------+--------------- +null | Berni | Patricio +F | Alejandro | Xinglin +M | Amabile | Zvonko +; + topHitsWithOneArgAndGroupBy schema::gender:s|first:s|last:s SELECT gender, FIRST(first_name) as first, LAST(first_name) as last FROM test_emp GROUP BY gender ORDER BY gender; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java index 6b1954f844ca7..d6e4c4fe07d7e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java @@ -149,6 +149,7 @@ protected Iterable.Batch> batches() { Batch aggregate = new Batch("Aggregation Rewrite", //new ReplaceDuplicateAggsWithReferences(), + new ReplaceMinMaxWithTopHits(), new ReplaceAggsWithMatrixStats(), new ReplaceAggsWithExtendedStats(), new ReplaceAggsWithStats(), From ac61c28a5256e4d3fd761a557f0e27c1cfedbe20 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 16 Apr 2019 16:04:27 +0300 Subject: [PATCH 2/3] Address comments --- .../sql/qa/src/main/resources/agg.csv-spec | 9 +++++++ .../sql/planner/QueryTranslatorTests.java | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec index 873a4e34aefbd..cadfb8b5d4ef0 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec @@ -415,6 +415,15 @@ false |90 |100 ; topHitsAsMinAndMax +schema::first:s|last:s +SELECT MIN(first_name) as first, MAX(first_name) as last FROM test_emp; + + first | last +---------------+------------- + Alejandro | Zvonko +; + +topHitsAsMinAndMaxAndGroupBy schema::gender:s|first:s|last:s SELECT gender, MIN(first_name) as first, MAX(first_name) as last FROM test_emp GROUP BY gender ORDER BY gender; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index c76e0da987d55..85bc20596e9e3 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -763,6 +763,18 @@ public void testTopHitsAggregationWithOneArg() { "\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," + "\"sort\":[{\"keyword\":{\"order\":\"asc\",\"missing\":\"_last\",\"unmapped_type\":\"keyword\"}}]}}}}}")); } + { + PhysicalPlan p = optimizeAndPlan("SELECT MIN(keyword) FROM test"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec eqe = (EsQueryExec) p; + assertEquals(1, eqe.output().size()); + assertEquals("MIN(keyword)", eqe.output().get(0).qualifiedName()); + assertEquals(DataType.KEYWORD, eqe.output().get(0).dataType()); + assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," + + "\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," + + "\"sort\":[{\"keyword\":{\"order\":\"asc\",\"missing\":\"_last\",\"unmapped_type\":\"keyword\"}}]}}}}}")); + } { PhysicalPlan p = optimizeAndPlan("SELECT LAST(date) FROM test"); assertEquals(EsQueryExec.class, p.getClass()); @@ -775,6 +787,18 @@ public void testTopHitsAggregationWithOneArg() { "\"explain\":false,\"docvalue_fields\":[{\"field\":\"date\",\"format\":\"epoch_millis\"}]," + "\"sort\":[{\"date\":{\"order\":\"desc\",\"missing\":\"_last\",\"unmapped_type\":\"date\"}}]}}}}}")); } + { + PhysicalPlan p = optimizeAndPlan("SELECT MAX(keyword) FROM test"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec eqe = (EsQueryExec) p; + assertEquals(1, eqe.output().size()); + assertEquals("MAX(keyword)", eqe.output().get(0).qualifiedName()); + assertEquals(DataType.KEYWORD, eqe.output().get(0).dataType()); + assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," + + "\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," + + "\"sort\":[{\"keyword\":{\"order\":\"desc\",\"missing\":\"_last\",\"unmapped_type\":\"keyword\"}}]}}}}}")); + } } public void testTopHitsAggregationWithTwoArgs() { From 7563e8945bacecf7cc3cb67f13c07e19de1e1719 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 16 Apr 2019 16:27:15 +0300 Subject: [PATCH 3/3] enhanced test --- .../sql/qa/src/main/resources/agg.csv-spec | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec index cadfb8b5d4ef0..5cc70a8cb5ef0 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec @@ -415,23 +415,23 @@ false |90 |100 ; topHitsAsMinAndMax -schema::first:s|last:s -SELECT MIN(first_name) as first, MAX(first_name) as last FROM test_emp; +schema::min:s|max:s|first:s|last:s +SELECT MIN(first_name) as min, MAX(first_name) as max, FIRST(first_name) as first, LAST(first_name) as last FROM test_emp; - first | last ----------------+------------- - Alejandro | Zvonko + min | max | first | last +---------------+---------------+--------------+---------- + Alejandro | Zvonko | Alejandro | Zvonko ; topHitsAsMinAndMaxAndGroupBy -schema::gender:s|first:s|last:s -SELECT gender, MIN(first_name) as first, MAX(first_name) as last FROM test_emp GROUP BY gender ORDER BY gender; - - gender | first | last ----------------+---------------+--------------- -null | Berni | Patricio -F | Alejandro | Xinglin -M | Amabile | Zvonko +schema::gender:s|min:s|max:s|first:s|last:s +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; + + gender | min | max | first | last +---------------+---------------+--------------+---------------+---------- +null | Berni | Patricio | Berni | Patricio +F | Alejandro | Xinglin | Alejandro | Xinglin +M | Amabile | Zvonko | Amabile | Zvonko ; topHitsWithOneArgAndGroupBy