From b0f5cbf944b6068b82a1a7bd4e1b49b9ab913b58 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Thu, 21 Nov 2019 11:09:36 -0800 Subject: [PATCH 1/5] Suppoted DISTINCT in select clause --- .../sql/domain/Field.java | 11 ++++++++++ .../sql/domain/MethodField.java | 8 ------- .../sql/parser/FieldMaker.java | 21 ++++++++++++++++++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java index a471f3654d..c4dd25bf17 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java @@ -16,6 +16,7 @@ package com.amazon.opendistroforelasticsearch.sql.domain; import com.alibaba.druid.sql.ast.SQLExpr; +import com.alibaba.druid.sql.ast.expr.SQLAggregateOption; import com.amazon.opendistroforelasticsearch.sql.parser.ChildrenType; import com.amazon.opendistroforelasticsearch.sql.parser.NestedType; @@ -38,12 +39,14 @@ public class Field implements Cloneable { private NestedType nested; private ChildrenType children; private SQLExpr expression; + private SQLAggregateOption option; public Field(String name, String alias) { this.name = name; this.alias = alias; this.nested = null; this.children = null; + this.option = null; } public Field(String name, String alias, NestedType nested, ChildrenType children) { @@ -104,6 +107,14 @@ public String getChildType() { return this.children.childType; } + public void setOption(SQLAggregateOption option) { + this.option = option; + } + + public SQLAggregateOption getOption() { + return option; + } + @Override public String toString() { return this.name; diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java index 18c39766c4..b6ac817f2d 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java @@ -68,14 +68,6 @@ public String toString() { return this.name + "(" + Util.joiner(params, ",") + ")"; } - public String getOption() { - return option; - } - - public void setOption(String option) { - this.option = option; - } - @Override public boolean isNested() { Map paramsAsMap = this.getParamsAsMap(); diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java index 0a4247f4e9..23f19a618d 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java @@ -17,6 +17,7 @@ import com.alibaba.druid.sql.ast.SQLExpr; import com.alibaba.druid.sql.ast.SQLObject; +import com.alibaba.druid.sql.ast.SQLSetQuantifier; import com.alibaba.druid.sql.ast.expr.SQLAggregateExpr; import com.alibaba.druid.sql.ast.expr.SQLAggregateOption; import com.alibaba.druid.sql.ast.expr.SQLAllColumnExpr; @@ -30,6 +31,9 @@ import com.alibaba.druid.sql.ast.expr.SQLPropertyExpr; import com.alibaba.druid.sql.ast.expr.SQLQueryExpr; import com.alibaba.druid.sql.ast.expr.SQLVariantRefExpr; +import com.alibaba.druid.sql.ast.statement.SQLSelectGroupByClause; +import com.alibaba.druid.sql.ast.statement.SQLSelectItem; +import com.alibaba.druid.sql.ast.statement.SQLSelectQueryBlock; import com.amazon.opendistroforelasticsearch.sql.domain.Field; import com.amazon.opendistroforelasticsearch.sql.domain.KVValue; import com.amazon.opendistroforelasticsearch.sql.domain.MethodField; @@ -57,6 +61,22 @@ public class FieldMaker { public Field makeField(SQLExpr expr, String alias, String tableAlias) throws SqlParseException { Field field = makeFieldImpl(expr, alias, tableAlias); + if (expr.getParent() != null && expr.getParent() instanceof SQLSelectItem + && expr.getParent().getParent() != null + && expr.getParent().getParent() instanceof SQLSelectQueryBlock) { + SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) expr.getParent().getParent(); + if (queryBlock.getDistionOption() == SQLSetQuantifier.DISTINCT) { + SQLAggregateOption option = SQLAggregateOption.DISTINCT; + field.setOption(option); + if (queryBlock.getGroupBy() == null) { + queryBlock.setGroupBy(new SQLSelectGroupByClause()); + } + SQLSelectGroupByClause groupByClause = queryBlock.getGroupBy(); + groupByClause.addItem(expr); + queryBlock.setGroupBy(groupByClause); + } + } + // why we may get null as a field??? if (field != null) { field.setExpression(expr); @@ -73,7 +93,6 @@ private Field makeFieldImpl(SQLExpr expr, String alias, String tableAlias) throw } else if (expr instanceof SQLBinaryOpExpr) { //make a SCRIPT method field; return makeFieldImpl(makeBinaryMethodField((SQLBinaryOpExpr) expr, alias, true), alias, tableAlias); - } else if (expr instanceof SQLAllColumnExpr) { return Field.STAR; } else if (expr instanceof SQLMethodInvokeExpr) { From 709af7d6b31aa82f7d20429eee395f43fddd71f6 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Thu, 21 Nov 2019 14:43:58 -0800 Subject: [PATCH 2/5] Added tests --- .../sql/esintgtest/AggregationIT.java | 17 +++++++ .../sql/unittest/AggregationOptionTest.java | 43 ++++++++++++++++++ .../sql/unittest/QueryFunctionsTest.java | 15 +------ .../sql/util/SqlExplainUtils.java | 45 +++++++++++++++++++ 4 files changed, 106 insertions(+), 14 deletions(-) create mode 100644 src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java create mode 100644 src/test/java/com/amazon/opendistroforelasticsearch/sql/util/SqlExplainUtils.java diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/AggregationIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/AggregationIT.java index cec2eef688..44572aa2e8 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/AggregationIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/AggregationIT.java @@ -1184,6 +1184,23 @@ public void groupByScriptedHistogram() throws Exception { Assert.assertThat(result, containsString("\"script\":{\"source\"")); } + @Test + public void distinctWithOneField() { + Assert.assertEquals( + executeQuery("SELECT DISTINCT name.lastname FROM " + TEST_INDEX_GAME_OF_THRONES, "jdbc"), + executeQuery("SELECT name.lastname FROM " + TEST_INDEX_GAME_OF_THRONES + + " GROUP BY name.lastname", "jdbc") + ); + } + + @Test + public void distinctWithMultipleFields() { + Assert.assertEquals( + executeQuery("SELECT DISTINCT age, gender FROM " + TEST_INDEX_ACCOUNT, "jdbc"), + executeQuery("SELECT age, gender FROM " + TEST_INDEX_ACCOUNT + + " GROUP BY age, gender", "jdbc") + ); + } private JSONObject getAggregation(final JSONObject queryResult, final String aggregationName) { diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java new file mode 100644 index 0000000000..c66ea7dd49 --- /dev/null +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.amazon.opendistroforelasticsearch.sql.unittest; + +import org.junit.Assert; +import org.junit.Test; + +import static com.amazon.opendistroforelasticsearch.sql.util.SqlExplainUtils.explain; + +/** + * Unit test class for feature of aggregation options: DISTINCT, ALL, UNIQUE, DEDUPLICATION + */ +public class AggregationOptionTest { + + @Test + public void distinctWithOneField() { + Assert.assertEquals( + explain("SELECT DISTINCT lastname FROM accounts"), + explain("SELECT lastname FROM accounts GROUP BY lastname") + ); + } + + @Test + public void distinctWithMultipleFields() { + Assert.assertEquals( + explain("SELECT DISTINCT city, age, balance FROM accounts"), + explain("SELECT city, age, balance FROM accounts GROUP BY city, age, balance") + ); + } +} diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/QueryFunctionsTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/QueryFunctionsTest.java index 011553a491..82db58ac03 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/QueryFunctionsTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/QueryFunctionsTest.java @@ -15,11 +15,9 @@ package com.amazon.opendistroforelasticsearch.sql.unittest; -import com.alibaba.druid.sql.parser.ParserException; import com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants; import com.amazon.opendistroforelasticsearch.sql.exception.SqlParseException; import com.amazon.opendistroforelasticsearch.sql.query.ESActionFactory; -import com.amazon.opendistroforelasticsearch.sql.query.QueryAction; import com.amazon.opendistroforelasticsearch.sql.util.CheckScriptContents; import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.client.Client; @@ -33,6 +31,7 @@ import java.sql.SQLFeatureNotSupportedException; +import static com.amazon.opendistroforelasticsearch.sql.util.SqlExplainUtils.explain; import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.matchPhraseQuery; import static org.elasticsearch.index.query.QueryBuilders.matchQuery; @@ -306,18 +305,6 @@ private String query(String sql) { return explain(sql); } - private String explain(String sql) { - try { - Client mockClient = Mockito.mock(Client.class); - CheckScriptContents.stubMockClient(mockClient); - QueryAction queryAction = ESActionFactory.create(mockClient, sql); - - return queryAction.explain().explain(); - } catch (SqlParseException | SQLFeatureNotSupportedException e) { - throw new ParserException("Illegal sql expr in: " + sql); - } - } - private Matcher contains(AbstractQueryBuilder queryBuilder) { return containsString(Strings.toString(queryBuilder, false, false)); } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/SqlExplainUtils.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/SqlExplainUtils.java new file mode 100644 index 0000000000..f44c01ef20 --- /dev/null +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/SqlExplainUtils.java @@ -0,0 +1,45 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.amazon.opendistroforelasticsearch.sql.util; + +import com.alibaba.druid.sql.parser.ParserException; +import com.amazon.opendistroforelasticsearch.sql.exception.SqlParseException; +import com.amazon.opendistroforelasticsearch.sql.query.ESActionFactory; +import com.amazon.opendistroforelasticsearch.sql.query.QueryAction; +import org.elasticsearch.client.Client; +import org.mockito.Mockito; + +import java.sql.SQLFeatureNotSupportedException; + +/** + * Test utils class that explains a query + */ +public class SqlExplainUtils { + + public static String explain(String query) { + try { + Client mockClient = Mockito.mock(Client.class); + CheckScriptContents.stubMockClient(mockClient); + QueryAction queryAction = ESActionFactory.create(mockClient, query); + + return queryAction.explain().explain(); + } catch (SqlParseException | SQLFeatureNotSupportedException e) { + throw new ParserException("Illegal sql expr in: " + query); + } + } + + private SqlExplainUtils() {} +} From cbf596a26129991d7b97d4e28667959019994012 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Thu, 21 Nov 2019 15:25:26 -0800 Subject: [PATCH 3/5] Addressed comments --- .../opendistroforelasticsearch/sql/domain/Field.java | 2 +- .../opendistroforelasticsearch/sql/parser/FieldMaker.java | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java index c4dd25bf17..65e7654ffc 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java @@ -107,7 +107,7 @@ public String getChildType() { return this.children.childType; } - public void setOption(SQLAggregateOption option) { + public void setAggregationOption(SQLAggregateOption option) { this.option = option; } diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java index 23f19a618d..ac44bdd2a1 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java @@ -61,13 +61,19 @@ public class FieldMaker { public Field makeField(SQLExpr expr, String alias, String tableAlias) throws SqlParseException { Field field = makeFieldImpl(expr, alias, tableAlias); + /** + * If a token of DISTINCT is in the SELECT clause, it will parsed and stored into the SQLSelectQueryBlock + * as a property of "DistionOpion" + * The following code block would be working only in the case of + * SELECT DISTINCT + fieldname (aka. SELECT DISTINCT(fieldname)) + */ if (expr.getParent() != null && expr.getParent() instanceof SQLSelectItem && expr.getParent().getParent() != null && expr.getParent().getParent() instanceof SQLSelectQueryBlock) { SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) expr.getParent().getParent(); if (queryBlock.getDistionOption() == SQLSetQuantifier.DISTINCT) { SQLAggregateOption option = SQLAggregateOption.DISTINCT; - field.setOption(option); + field.setAggregationOption(option); if (queryBlock.getGroupBy() == null) { queryBlock.setGroupBy(new SQLSelectGroupByClause()); } From 255b7536466d3fc5369c62e90b4432b2431cb57c Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Fri, 22 Nov 2019 13:44:17 -0800 Subject: [PATCH 4/5] Addressed comments --- .../sql/domain/Field.java | 2 +- .../sql/domain/MethodField.java | 4 +- .../sql/domain/ScriptMethodField.java | 4 +- .../sql/parser/FieldMaker.java | 45 +++++++++---------- 4 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java index 65e7654ffc..3ca8cca3a9 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/Field.java @@ -35,11 +35,11 @@ public class Field implements Cloneable { public static final Field STAR = new Field("*", ""); protected String name; + protected SQLAggregateOption option; private String alias; private NestedType nested; private ChildrenType children; private SQLExpr expression; - private SQLAggregateOption option; public Field(String name, String alias) { this.name = name; diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java index b6ac817f2d..f41f67ef5d 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/MethodField.java @@ -15,6 +15,7 @@ package com.amazon.opendistroforelasticsearch.sql.domain; +import com.alibaba.druid.sql.ast.expr.SQLAggregateOption; import com.amazon.opendistroforelasticsearch.sql.parser.NestedType; import com.amazon.opendistroforelasticsearch.sql.utils.Util; @@ -29,9 +30,8 @@ */ public class MethodField extends Field { private List params = null; - private String option; - public MethodField(String name, List params, String option, String alias) { + public MethodField(String name, List params, SQLAggregateOption option, String alias) { super(name, alias); this.params = params; this.option = option; diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/ScriptMethodField.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/ScriptMethodField.java index 4d12d09422..43720b1071 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/ScriptMethodField.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/domain/ScriptMethodField.java @@ -15,6 +15,8 @@ package com.amazon.opendistroforelasticsearch.sql.domain; +import com.alibaba.druid.sql.ast.expr.SQLAggregateOption; + import java.util.List; /** @@ -23,7 +25,7 @@ public class ScriptMethodField extends MethodField { private final String functionName; - public ScriptMethodField(String functionName, List params, String option, String alias) { + public ScriptMethodField(String functionName, List params, SQLAggregateOption option, String alias) { super("script", params, option, alias); this.functionName = functionName; } diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java index ac44bdd2a1..f149f99d54 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java @@ -60,28 +60,7 @@ public class FieldMaker { public Field makeField(SQLExpr expr, String alias, String tableAlias) throws SqlParseException { Field field = makeFieldImpl(expr, alias, tableAlias); - - /** - * If a token of DISTINCT is in the SELECT clause, it will parsed and stored into the SQLSelectQueryBlock - * as a property of "DistionOpion" - * The following code block would be working only in the case of - * SELECT DISTINCT + fieldname (aka. SELECT DISTINCT(fieldname)) - */ - if (expr.getParent() != null && expr.getParent() instanceof SQLSelectItem - && expr.getParent().getParent() != null - && expr.getParent().getParent() instanceof SQLSelectQueryBlock) { - SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) expr.getParent().getParent(); - if (queryBlock.getDistionOption() == SQLSetQuantifier.DISTINCT) { - SQLAggregateOption option = SQLAggregateOption.DISTINCT; - field.setAggregationOption(option); - if (queryBlock.getGroupBy() == null) { - queryBlock.setGroupBy(new SQLSelectGroupByClause()); - } - SQLSelectGroupByClause groupByClause = queryBlock.getGroupBy(); - groupByClause.addItem(expr); - queryBlock.setGroupBy(groupByClause); - } - } + addGroupByForDistinctFieldsInSelect(expr, field); // why we may get null as a field??? if (field != null) { @@ -154,6 +133,24 @@ private Field makeFieldImpl(SQLExpr expr, String alias, String tableAlias) throw } } + private void addGroupByForDistinctFieldsInSelect(SQLExpr expr, Field field) { + if (expr.getParent() != null && expr.getParent() instanceof SQLSelectItem + && expr.getParent().getParent() != null + && expr.getParent().getParent() instanceof SQLSelectQueryBlock) { + SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) expr.getParent().getParent(); + if (queryBlock.getDistionOption() == SQLSetQuantifier.DISTINCT) { + SQLAggregateOption option = SQLAggregateOption.DISTINCT; + field.setAggregationOption(option); + if (queryBlock.getGroupBy() == null) { + queryBlock.setGroupBy(new SQLSelectGroupByClause()); + } + SQLSelectGroupByClause groupByClause = queryBlock.getGroupBy(); + groupByClause.addItem(expr); + queryBlock.setGroupBy(groupByClause); + } + } + } + private static Object getScriptValue(SQLExpr expr) throws SqlParseException { return Util.getScriptValue(expr); } @@ -403,9 +400,9 @@ public MethodField makeMethodField(String name, List arguments, SQLAggr } if (builtInScriptFunction) { - return new ScriptMethodField(name, paramers, option == null ? null : option.name(), alias); + return new ScriptMethodField(name, paramers, option, alias); } else { - return new MethodField(name, paramers, option == null ? null : option.name(), alias); + return new MethodField(name, paramers, option, alias); } } } From a4fdc409214c6a9d1878c6b0ecc2a55a866fe249 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Fri, 22 Nov 2019 15:10:03 -0800 Subject: [PATCH 5/5] Addressed comments --- .../sql/unittest/AggregationOptionTest.java | 60 +++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java index c66ea7dd49..21653b7689 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/AggregationOptionTest.java @@ -15,10 +15,17 @@ package com.amazon.opendistroforelasticsearch.sql.unittest; +import com.alibaba.druid.sql.ast.expr.SQLAggregateOption; +import com.alibaba.druid.sql.ast.expr.SQLQueryExpr; +import com.amazon.opendistroforelasticsearch.sql.domain.Field; +import com.amazon.opendistroforelasticsearch.sql.domain.Select; +import com.amazon.opendistroforelasticsearch.sql.exception.SqlParseException; +import com.amazon.opendistroforelasticsearch.sql.parser.SqlParser; +import com.amazon.opendistroforelasticsearch.sql.util.SqlParserUtils; import org.junit.Assert; import org.junit.Test; -import static com.amazon.opendistroforelasticsearch.sql.util.SqlExplainUtils.explain; +import java.util.List; /** * Unit test class for feature of aggregation options: DISTINCT, ALL, UNIQUE, DEDUPLICATION @@ -26,18 +33,49 @@ public class AggregationOptionTest { @Test - public void distinctWithOneField() { - Assert.assertEquals( - explain("SELECT DISTINCT lastname FROM accounts"), - explain("SELECT lastname FROM accounts GROUP BY lastname") - ); + public void selectDistinctFieldsShouldHaveAggregationOption() { + List fields = getSelectFields("SELECT DISTINCT gender, city FROM accounts"); + for (Field field: fields) { + Assert.assertEquals(field.getOption(), SQLAggregateOption.DISTINCT); + } } @Test - public void distinctWithMultipleFields() { - Assert.assertEquals( - explain("SELECT DISTINCT city, age, balance FROM accounts"), - explain("SELECT city, age, balance FROM accounts GROUP BY city, age, balance") - ); + public void selectWithoutDistinctFieldsShouldNotHaveAggregationOption() { + List fields = getSelectFields("SELECT gender, city FROM accounts"); + for (Field field: fields) { + Assert.assertNull(field.getOption()); + } + } + + @Test + public void selectDistinctWithoutGroupByShouldHaveGroupByItems() { + List> groupBys = getGroupBys("SELECT DISTINCT gender, city FROM accounts"); + Assert.assertFalse(groupBys.isEmpty()); + } + + @Test + public void selectWithoutDistinctWithoutGroupByShouldNotHaveGroupByItems() { + List> groupBys = getGroupBys("SELECT gender, city FROM accounts"); + Assert.assertTrue(groupBys.isEmpty()); + } + + private List> getGroupBys(String query) { + return getSelect(query).getGroupBys(); + } + + private List getSelectFields(String query) { + return getSelect(query).getFields(); + } + + private Select getSelect(String query) { + SQLQueryExpr queryExpr = SqlParserUtils.parse(query); + Select select = null; + try { + select = new SqlParser().parseSelect(queryExpr); + } catch (SqlParseException e) { + e.printStackTrace(); + } + return select; } }