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

Commit

Permalink
Better error reporting for unsupported case like max(x)-min(y) (#286)
Browse files Browse the repository at this point in the history 
* Better error reporting for unsupported case like max(x)-min(y)
* Even more readable errors
  • Loading branch information
@galkk
galkk authored Nov 15, 2019
1 parent 763ae7e commit e0f8702
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 4 deletions.
18 changes: 18 additions & 0 deletions src/main/java/com/amazon/opendistroforelasticsearch/sql/parser/FieldMaker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.amazon.opendistroforelasticsearch.sql.parser;

import com.alibaba.druid.sql.ast.SQLExpr;
import com.alibaba.druid.sql.ast.SQLObject;
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 @@ -34,6 +35,7 @@
import com.amazon.opendistroforelasticsearch.sql.domain.MethodField;
import com.amazon.opendistroforelasticsearch.sql.domain.ScriptMethodField;
import com.amazon.opendistroforelasticsearch.sql.domain.Where;
import com.amazon.opendistroforelasticsearch.sql.exception.SqlFeatureNotImplementedException;
import com.amazon.opendistroforelasticsearch.sql.exception.SqlParseException;
import com.amazon.opendistroforelasticsearch.sql.utils.SQLFunctions;
import com.amazon.opendistroforelasticsearch.sql.utils.Util;
Expand Down Expand Up @@ -230,6 +232,7 @@ private static SQLMethodInvokeExpr convertBinaryOperatorToMethod(String operator
SQLMethodInvokeExpr methodInvokeExpr = new SQLMethodInvokeExpr(operator, null);
methodInvokeExpr.addParameter(expr.getLeft());
methodInvokeExpr.addParameter(expr.getRight());
methodInvokeExpr.putAttribute("source", expr);
return methodInvokeExpr;
}

Expand Down Expand Up @@ -317,6 +320,21 @@ public MethodField makeMethodField(String name, List<SQLExpr> arguments, SQLAggr
} else if (object instanceof SQLCastExpr) {
String scriptCode = new CastParser((SQLCastExpr) object, alias, tableAlias).parse(false);
paramers.add(new KVValue("script", new SQLCharExpr(scriptCode)));
} else if (object instanceof SQLAggregateExpr) {
SQLObject parent = object.getParent();
SQLExpr source = (SQLExpr) parent.getAttribute("source");

if (parent instanceof SQLMethodInvokeExpr && source == null) {
throw new SqlFeatureNotImplementedException(
"Function calls of form '"
+ ((SQLMethodInvokeExpr) parent).getMethodName()
+ "("
+ ((SQLAggregateExpr) object).getMethodName()
+ "(...))' are not implemented yet");
}

throw new SqlFeatureNotImplementedException(
"The complex aggregate expressions are not implemented yet: " + source);
} else {
paramers.add(new KVValue(Util.removeTableAilasFromField(object, tableAlias)));
}
Expand Down
83 changes: 79 additions & 4 deletions src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/parser/SqlParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,24 @@
import com.amazon.opendistroforelasticsearch.sql.domain.hints.Hint;
import com.amazon.opendistroforelasticsearch.sql.domain.hints.HintType;
import com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants;
import com.amazon.opendistroforelasticsearch.sql.exception.SqlFeatureNotImplementedException;
import com.amazon.opendistroforelasticsearch.sql.exception.SqlParseException;
import com.amazon.opendistroforelasticsearch.sql.parser.ElasticSqlExprParser;
import com.amazon.opendistroforelasticsearch.sql.parser.ScriptFilter;
import com.amazon.opendistroforelasticsearch.sql.parser.SqlParser;
import com.amazon.opendistroforelasticsearch.sql.query.AggregationQueryAction;
import com.amazon.opendistroforelasticsearch.sql.query.maker.QueryMaker;
import com.amazon.opendistroforelasticsearch.sql.query.multi.MultiQuerySelect;
import com.amazon.opendistroforelasticsearch.sql.util.CheckScriptContents;
import org.elasticsearch.client.Client;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.util.LinkedList;
import java.util.List;
Expand All @@ -56,16 +62,20 @@
import static com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants.TEST_INDEX_GAME_OF_THRONES;
import static com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants.TEST_INDEX_ODBC;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.mock;

public class SqlParserTest {

private static SqlParser parser;
private SqlParser parser;

@BeforeClass
public static void init() {
@Before
public void init() {
parser = new SqlParser();
}

@Rule
public ExpectedException thrown= ExpectedException.none();

@Test
public void whereConditionLeftFunctionRightPropertyGreatTest() throws Exception {

Expand All @@ -86,6 +96,71 @@ public void whereConditionLeftFunctionRightPropertyGreatTest() throws Exception
Assert.assertTrue(matcher.find());
}

@Test()
public void failingQueryTest() throws SqlParseException {
thrown.expect(SqlFeatureNotImplementedException.class);
thrown.expectMessage(
"The complex aggregate expressions are not implemented yet: MAX(FlightDelayMin) - MIN(FlightDelayMin)");

Select select =
parser.parseSelect((SQLQueryExpr) queryToExpr(
"SELECT DestCountry, dayOfWeek, max(FlightDelayMin) - min(FlightDelayMin)" +
" FROM kibana_sample_data_flights\n" +
" GROUP BY DestCountry, dayOfWeek\n"));

AggregationQueryAction queryAction = new AggregationQueryAction(mock(Client.class), select);
String elasticDsl = queryAction.explain().explain();
}

@Test()
public void failingQueryTest2() throws SqlParseException {
thrown.expect(SqlFeatureNotImplementedException.class);
thrown.expectMessage(
"Function calls of form 'log(MAX(...))' are not implemented yet");

Select select =
parser.parseSelect((SQLQueryExpr) queryToExpr(
"SELECT DestCountry, dayOfWeek, log(max(FlightDelayMin))" +
" FROM kibana_sample_data_flights\n" +
" GROUP BY DestCountry, dayOfWeek\n"));

AggregationQueryAction queryAction = new AggregationQueryAction(mock(Client.class), select);
String elasticDsl = queryAction.explain().explain();
}

@Test()
public void failingQueryWithHavingTest() throws SqlParseException {
thrown.expect(SqlFeatureNotImplementedException.class);
thrown.expectMessage(
"The complex aggregate expressions are not implemented yet: MAX(FlightDelayMin) - MIN(FlightDelayMin)");

Select select =
parser.parseSelect((SQLQueryExpr) queryToExpr(
"SELECT DestCountry, dayOfWeek, max(FlightDelayMin) - min(FlightDelayMin) " +
" FROM kibana_sample_data_flights\n" +
" GROUP BY DestCountry, dayOfWeek\n" +
" HAVING max(FlightDelayMin) - min(FlightDelayMin)) * count(FlightDelayMin) + 14 > 100"));

AggregationQueryAction queryAction = new AggregationQueryAction(mock(Client.class), select);
String elasticDsl = queryAction.explain().explain();
}

@Test()
@Ignore("Github issues: https://github.com/opendistro-for-elasticsearch/sql/issues/194, " +
"https://github.com/opendistro-for-elasticsearch/sql/issues/234")
public void failingQueryWithHavingTest2() throws SqlParseException {
Select select =
parser.parseSelect((SQLQueryExpr) queryToExpr(
"SELECT DestCountry, dayOfWeek, max(FlightDelayMin) " +
" FROM kibana_sample_data_flights\n" +
" GROUP BY DestCountry, dayOfWeek\n" +
" HAVING max(FlightDelayMin) - min(FlightDelayMin) > 100"));

AggregationQueryAction queryAction = new AggregationQueryAction(mock(Client.class), select);

String elasticDsl = queryAction.explain().explain();
}

@Test
public void whereConditionLeftFunctionRightFunctionEqualTest() throws Exception {

Expand Down

0 comments on commit e0f8702

Please sign in to comment.