From a2338bb116f40a94375e61b587eeab4ddb57e432 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Wed, 28 Nov 2018 14:29:10 +0100 Subject: [PATCH] SQL: Fix issue with wrong data type for scripted Grouping keys (#35969) When the grouping key of a GROUP BY is a painless script (functions are involved), the data type of the key was incorrect in certain cases (Boolean, IP, Date). This resulted in returning wrong data type for this columns in the query results. E.g.: ``` SELECT COUNT(*), a > 10 AS a FROM t GROUP BY a ``` Fixes: #35662 --- .../sql/querydsl/agg/GroupByScriptKey.java | 7 ++ .../xpack/sql/planner/QueryFolderTests.java | 86 +++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByScriptKey.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByScriptKey.java index 910f2a2a763..9c907be38da 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByScriptKey.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByScriptKey.java @@ -9,6 +9,7 @@ import org.elasticsearch.search.aggregations.bucket.composite.TermsValuesSourceB import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.sql.querydsl.container.Sort.Direction; +import org.elasticsearch.xpack.sql.type.DataType; import java.util.Objects; @@ -45,6 +46,12 @@ public class GroupByScriptKey extends GroupByKey { builder.valueType(ValueType.DOUBLE); } else if (script.outputType().isString()) { builder.valueType(ValueType.STRING); + } else if (script.outputType() == DataType.DATE) { + builder.valueType(ValueType.DATE); + } else if (script.outputType() == DataType.BOOLEAN) { + builder.valueType(ValueType.BOOLEAN); + } else if (script.outputType() == DataType.IP) { + builder.valueType(ValueType.IP); } return builder; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java index c8de560787b..998fc132f98 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java @@ -26,6 +26,7 @@ import org.junit.BeforeClass; import java.util.Map; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.startsWith; public class QueryFolderTests extends ESTestCase { @@ -177,4 +178,89 @@ public class QueryFolderTests extends ESTestCase { assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); assertThat(ee.output().get(1).toString(), startsWith("MAX(int){a->")); } + + public void testGroupKeyTypes_Boolean() { + PhysicalPlan p = plan("SELECT count(*), int > 10 AS a FROM test GROUP BY a"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertThat(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("{\"script\":{" + + "\"source\":\"InternalSqlScriptUtils.gt(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1)\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":\"int\",\"v1\":10}},\"missing_bucket\":true," + + "\"value_type\":\"boolean\",\"order\":\"asc\"}}}]}}}")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->")); + assertThat(ee.output().get(1).toString(), startsWith("a{s->")); + } + + public void testGroupKeyTypes_Integer() { + PhysicalPlan p = plan("SELECT count(*), int + 10 AS a FROM test GROUP BY a"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertThat(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("{\"script\":{" + + "\"source\":\"InternalSqlScriptUtils.add(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1)\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":\"int\",\"v1\":10}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->")); + assertThat(ee.output().get(1).toString(), startsWith("a{s->")); + } + + public void testGroupKeyTypes_Rational() { + PhysicalPlan p = plan("SELECT count(*), sin(int) AS a FROM test GROUP BY a"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertThat(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("{\"script\":{" + + "\"source\":\"InternalSqlScriptUtils.sin(InternalSqlScriptUtils.docValue(doc,params.v0))\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":\"int\"}},\"missing_bucket\":true," + + "\"value_type\":\"double\",\"order\":\"asc\"}}}]}}}")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->")); + assertThat(ee.output().get(1).toString(), startsWith("a{s->")); + } + + public void testGroupKeyTypes_String() { + PhysicalPlan p = plan("SELECT count(*), LCASE(keyword) AS a FROM test GROUP BY a"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertThat(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("{\"script\":{" + + "\"source\":\"InternalSqlScriptUtils.lcase(InternalSqlScriptUtils.docValue(doc,params.v0))\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":\"keyword\"}},\"missing_bucket\":true," + + "\"value_type\":\"string\",\"order\":\"asc\"}}}]}}}")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->")); + assertThat(ee.output().get(1).toString(), startsWith("a{s->")); + } + + public void testGroupKeyTypes_IP() { + PhysicalPlan p = plan("SELECT count(*), CAST(keyword AS IP) AS a FROM test GROUP BY a"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertThat(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("{\"script\":{" + + "\"source\":\"InternalSqlScriptUtils.docValue(doc,params.v0)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"keyword\"}},\"missing_bucket\":true," + + "\"value_type\":\"ip\",\"order\":\"asc\"}}}]}}}")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->")); + assertThat(ee.output().get(1).toString(), startsWith("a{s->")); + } + + public void testGroupKeyTypes_Date() { + PhysicalPlan p = plan("SELECT count(*), date + INTERVAL '1-2' YEAR TO MONTH AS a FROM test GROUP BY a"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertThat(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), + endsWith("{\"script\":{" + + "\"source\":\"InternalSqlScriptUtils.add(InternalSqlScriptUtils.docValue(doc,params.v0)," + + "InternalSqlScriptUtils.intervalYearMonth(params.v1,params.v2))\",\"lang\":\"painless\",\"params\":{" + + "\"v0\":\"date\",\"v1\":\"P1Y2M\",\"v2\":\"INTERVAL_YEAR_TO_MONTH\"}},\"missing_bucket\":true," + + "\"value_type\":\"date\",\"order\":\"asc\"}}}]}}}")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->")); + assertThat(ee.output().get(1).toString(), startsWith("a{s->")); + } }