Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Support DISTINCT feature in SELECT clause #300

Merged
merged 5 commits into from
Nov 22, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -104,6 +107,14 @@ public String getChildType() {
return this.children.childType;
}

public void setOption(SQLAggregateOption option) {
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
this.option = option;
}

public SQLAggregateOption getOption() {
return option;
}

@Override
public String toString() {
return this.name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,6 @@ public String toString() {
return this.name + "(" + Util.joiner(params, ",") + ")";
}

public String getOption() {
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
return option;
}

public void setOption(String option) {
this.option = option;
}

@Override
public boolean isNested() {
Map<String, Object> paramsAsMap = this.getParamsAsMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
&& 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);
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,23 @@ public void groupByScriptedHistogram() throws Exception {
Assert.assertThat(result, containsString("\"script\":{\"source\""));
}

@Test
public void distinctWithOneField() {
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
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)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved

@Test
public void distinctWithOneField() {
Assert.assertEquals(
explain("SELECT DISTINCT lastname FROM accounts"),
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
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")
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String> contains(AbstractQueryBuilder queryBuilder) {
return containsString(Strings.toString(queryBuilder, false, false));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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() {}
}