diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java index d97a2517e2..379995ce5c 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java @@ -16,6 +16,8 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_TEXT_KEYWORD; + import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; @@ -60,8 +62,8 @@ public QueryBuilder build(FunctionExpression func) { * from reference and literal in function arguments. * * @param fieldName field name - * @param fieldType expr fieldType - * @param literal expr literal + * @param fieldType field type + * @param literal field value literal * @return query */ protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal) { @@ -69,4 +71,19 @@ protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue l "Subclass doesn't implement this and build method either"); } + /** + * Convert multi-field text field name to its inner keyword field. The limitation and assumption + * is that the keyword field name is always "keyword" which is true by default. + * + * @param fieldName field name + * @param fieldType field type + * @return keyword field name for multi-field, otherwise original field name returned + */ + protected String convertTextToKeyword(String fieldName, ExprType fieldType) { + if (fieldType == ES_TEXT_KEYWORD) { + return fieldName + ".keyword"; + } + return fieldName; + } + } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java index 01e3c4d9f0..db07116a25 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java @@ -16,8 +16,6 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene; -import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_TEXT_KEYWORD; - import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import org.elasticsearch.index.query.QueryBuilder; @@ -30,9 +28,7 @@ public class TermQuery extends LuceneQuery { @Override protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal) { - if (fieldType == ES_TEXT_KEYWORD) { // Assume inner field name is always "keyword" - fieldName += ".keyword"; - } + fieldName = convertTextToKeyword(fieldName, fieldType); return QueryBuilders.termQuery(fieldName, literal.value()); } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/WildcardQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/WildcardQuery.java index 1f20663a9d..7b5c109924 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/WildcardQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/WildcardQuery.java @@ -28,6 +28,7 @@ public class WildcardQuery extends LuceneQuery { @Override protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal) { + fieldName = convertTextToKeyword(fieldName, fieldType); String matchText = convertSqlWildcardToLucene(literal.stringValue()); return QueryBuilders.wildcardQuery(fieldName, matchText); } diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index 5a7d93ec6b..aabbd857dc 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -171,7 +171,7 @@ void should_build_script_query_for_comparison_between_fields() { @Test void should_build_bool_query_for_and_or_expression() { String[] names = { "filter", "should" }; - FunctionExpression expr1 = dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")); + FunctionExpression expr1 = dsl.equal(ref("name", STRING), literal("John")); FunctionExpression expr2 = dsl.equal(ref("age", INTEGER), literal(30)); Expression[] exprs = { dsl.and(expr1, expr2), @@ -185,7 +185,7 @@ void should_build_bool_query_for_and_or_expression() { + " \"" + names[i] + "\" : [\n" + " {\n" + " \"term\" : {\n" - + " \"name.keyword\" : {\n" + + " \"name\" : {\n" + " \"value\" : \"John\",\n" + " \"boost\" : 1.0\n" + " }\n" @@ -233,6 +233,38 @@ void should_build_bool_query_for_not_expression() { ref("age", INTEGER), literal(30))))); } + @Test + void should_use_keyword_for_multi_field_in_equality_expression() { + assertEquals( + "{\n" + + " \"term\" : {\n" + + " \"name.keyword\" : {\n" + + " \"value\" : \"John\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.equal( + ref("name", ES_TEXT_KEYWORD), literal("John")))); + } + + @Test + void should_use_keyword_for_multi_field_in_like_expression() { + assertEquals( + "{\n" + + " \"wildcard\" : {\n" + + " \"name.keyword\" : {\n" + + " \"wildcard\" : \"John*\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.like( + ref("name", ES_TEXT_KEYWORD), literal("John%")))); + } + private String buildQuery(Expression expr) { return filterQueryBuilder.build(expr).toString(); } diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 974973fe49..622497dcf2 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -114,6 +114,9 @@ task integTestWithNewEngine(type: RestIntegTestTask) { exclude 'com/amazon/opendistroforelasticsearch/sql/doctest/**/*IT.class' exclude 'com/amazon/opendistroforelasticsearch/sql/correctness/**' + + // Skip old semantic analyzer IT because analyzer in new engine has different behavior + exclude 'com/amazon/opendistroforelasticsearch/sql/legacy/QueryAnalysisIT.class' } } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java index 58c10073a1..e069141d4b 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java @@ -32,7 +32,7 @@ public class SQLCorrectnessIT extends CorrectnessTestBase { private static final String ROOT_DIR = "correctness/"; private static final String[] EXPR_TEST_DIR = { "expressions" }; - private static final String[] QUERY_TEST_DIR = { "queries"/*, "bugfixes"*/ }; //TODO: skip bugfixes folder for now since it fails + private static final String[] QUERY_TEST_DIR = { "queries", "bugfixes" }; @Override protected void init() throws Exception { @@ -71,6 +71,4 @@ private void verifyQueries(Path file, Function converter) { } } - - } diff --git a/integ-test/src/test/resources/correctness/bugfixes/234.txt b/integ-test/src/test/resources/correctness/bugfixes/234.txt new file mode 100644 index 0000000000..3056acb199 --- /dev/null +++ b/integ-test/src/test/resources/correctness/bugfixes/234.txt @@ -0,0 +1,2 @@ +SELECT FlightNum FROM kibana_sample_data_flights where (AvgTicketPrice + 100) <= 1000 +SELECT FlightNum FROM kibana_sample_data_flights where ROUND(FlightTimeMin) > ABS(FlightDelayMin) \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/bugfixes/237.txt b/integ-test/src/test/resources/correctness/bugfixes/237.txt new file mode 100644 index 0000000000..5ffd41287c --- /dev/null +++ b/integ-test/src/test/resources/correctness/bugfixes/237.txt @@ -0,0 +1,3 @@ +SELECT ((Origin = 'Munich Airport') AND (Dest = 'Venice Marco Polo Airport')) AS Calculation_462181953506873347 FROM kibana_sample_data_flights +SELECT ((Origin = 'Munich Airport') OR (Origin = 'Itami Airport')) AS Calculation_462181953506873347 FROM kibana_sample_data_flights +SELECT NOT (Origin = 'Munich Airport') AS Calculation_462181953506873347 FROM kibana_sample_data_flights \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/bugfixes/368.txt b/integ-test/src/test/resources/correctness/bugfixes/368.txt new file mode 100644 index 0000000000..a81edeec97 --- /dev/null +++ b/integ-test/src/test/resources/correctness/bugfixes/368.txt @@ -0,0 +1,2 @@ +SELECT Origin FROM kibana_sample_data_flights WHERE Origin LIKE 'London Hea%' +SELECT Origin FROM kibana_sample_data_flights WHERE Origin LIKE '%International%' \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/bugfixes/521.txt b/integ-test/src/test/resources/correctness/bugfixes/521.txt index 72a27b01d7..f0ef35d198 100644 --- a/integ-test/src/test/resources/correctness/bugfixes/521.txt +++ b/integ-test/src/test/resources/correctness/bugfixes/521.txt @@ -1 +1 @@ -SELECT timestamp, COUNT(*) FROM kibana_sample_data_flights GROUP BY timestamp \ No newline at end of file +SELECT timestamp FROM kibana_sample_data_flights GROUP BY timestamp \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/bugfixes/690.txt b/integ-test/src/test/resources/correctness/bugfixes/690.txt new file mode 100644 index 0000000000..940087e79e --- /dev/null +++ b/integ-test/src/test/resources/correctness/bugfixes/690.txt @@ -0,0 +1,4 @@ +SELECT FlightNum, Origin FROM kibana_sample_data_flights WHERE NULL IS NULL +SELECT FlightNum, Origin FROM kibana_sample_data_flights WHERE NULL IS NOT NULL +SELECT FlightNum, Origin FROM kibana_sample_data_flights WHERE NULL IS NULL AND NULL IS NULL +SELECT FlightNum, Origin FROM kibana_sample_data_flights WHERE NULL IS NULL OR NULL IS NULL diff --git a/integ-test/src/test/resources/correctness/expressions/predicates.txt b/integ-test/src/test/resources/correctness/expressions/predicates.txt index e69de29bb2..9bc00c0be9 100644 --- a/integ-test/src/test/resources/correctness/expressions/predicates.txt +++ b/integ-test/src/test/resources/correctness/expressions/predicates.txt @@ -0,0 +1,7 @@ +true AND true AS bool +false AND true AS bool +false OR false AS bool +true or false AS bool +NOT true AS bool +NOT false AS bool +NOT (true AND false) AS bool \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/kibana_sample_data_flights.json b/integ-test/src/test/resources/correctness/kibana_sample_data_flights.json index d180b6283c..4ad67e88a6 100644 --- a/integ-test/src/test/resources/correctness/kibana_sample_data_flights.json +++ b/integ-test/src/test/resources/correctness/kibana_sample_data_flights.json @@ -11,7 +11,13 @@ "type": "keyword" }, "Dest": { - "type": "keyword" + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } }, "DestAirportID": { "type": "keyword" @@ -53,7 +59,13 @@ "type": "float" }, "Origin": { - "type": "keyword" + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } }, "OriginAirportID": { "type": "keyword" diff --git a/integ-test/src/test/resources/correctness/queries/select.txt b/integ-test/src/test/resources/correctness/queries/select.txt index 298e0f22a1..50372e4ff2 100644 --- a/integ-test/src/test/resources/correctness/queries/select.txt +++ b/integ-test/src/test/resources/correctness/queries/select.txt @@ -6,3 +6,10 @@ SELECT abs(DistanceMiles), Abs(FlightDelayMin) FROM kibana_sample_data_flights SELECT Cancelled AS Cancel, AvgTicketPrice AS ATP FROM kibana_sample_data_flights SELECT Cancelled AS `Cancel`, AvgTicketPrice AS "ATP" FROM kibana_sample_data_flights SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE AvgTicketPrice <= 500 +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE NOT AvgTicketPrice <= 500 +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE AvgTicketPrice <= 500 AND FlightDelayMin = 0 +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE AvgTicketPrice <= 500 OR FlightDelayMin = 0 +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE AvgTicketPrice + 100 <= 500 +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE ABS(AvgTicketPrice * -2) > 1000 +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE Carrier LIKE 'JetBeat_' +SELECT AvgTicketPrice, Carrier FROM kibana_sample_data_flights WHERE Carrier LIKE '%Air%' diff --git a/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryActionTest.java b/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryActionTest.java index dac792ef06..31e4b8845b 100644 --- a/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryActionTest.java +++ b/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryActionTest.java @@ -71,8 +71,8 @@ public void skipExplainThatNotSupport() { @Test public void skipQueryThatNotSupport() { SQLQueryRequest request = new SQLQueryRequest( - new JSONObject("{\"query\": \"SELECT * FROM test WHERE age = 10\"}"), - "SELECT * FROM test WHERE age = 10", + new JSONObject("{\"query\": \"SELECT * FROM test WHERE age = 10 GROUP BY age\"}"), + "SELECT * FROM test WHERE age = 10 GROUP BY age", QUERY_API_ENDPOINT, ""); diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index ac72d0b233..3bc728ad03 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -77,8 +77,12 @@ selectElement fromClause : FROM tableName + (whereClause)? ; +whereClause + : WHERE expression + ; // Literals @@ -142,7 +146,10 @@ timestampLiteral // Simplified approach for expression expression - : predicate #predicateExpression + : NOT expression #notExpression + | left=expression AND right=expression #andExpression + | left=expression OR right=expression #orExpression + | predicate #predicateExpression ; predicate diff --git a/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilder.java b/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilder.java index 2d00909967..b404062261 100644 --- a/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilder.java +++ b/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilder.java @@ -20,10 +20,12 @@ import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.SelectClauseContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.SelectElementContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.SimpleSelectContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.WhereClauseContext; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Alias; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; @@ -90,7 +92,16 @@ public UnresolvedPlan visitSelectClause(SelectClauseContext ctx) { @Override public UnresolvedPlan visitFromClause(FromClauseContext ctx) { - return new Relation(visitAstExpression(ctx.tableName().qualifiedName())); + UnresolvedPlan result = new Relation(visitAstExpression(ctx.tableName().qualifiedName())); + if (ctx.whereClause() != null) { + result = visit(ctx.whereClause()).attach(result); + } + return result; + } + + @Override + public UnresolvedPlan visitWhereClause(WhereClauseContext ctx) { + return new Filter(visitAstExpression(ctx.expression())); } @Override diff --git a/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java index 1dadde63ff..11018fe9d3 100644 --- a/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java +++ b/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java @@ -20,22 +20,34 @@ import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.IS_NULL; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.LIKE; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT_LIKE; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BinaryComparisonPredicateContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BooleanContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.DateLiteralContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.IsNullPredicateContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.LikePredicateContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.MathExpressionAtomContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.NotExpressionContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.NullLiteralContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.ScalarFunctionCallContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.SignedDecimalContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.SignedRealContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.StringContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.TimeLiteralContext; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.TimestampLiteralContext; import com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Not; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Or; import com.amazon.opendistroforelasticsearch.sql.ast.expression.QualifiedName; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; import com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils; -import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser; +import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.AndExpressionContext; import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.ColumnNameContext; import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.IdentContext; import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.NestedExpressionAtomContext; +import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.OrExpressionContext; import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.QualifiedNameContext; import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.TableNameContext; import com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParserBaseVisitor; @@ -96,7 +108,7 @@ public UnresolvedExpression visitScalarFunctionCall(ScalarFunctionCallContext ct } @Override - public UnresolvedExpression visitIsNullPredicate(OpenDistroSQLParser.IsNullPredicateContext ctx) { + public UnresolvedExpression visitIsNullPredicate(IsNullPredicateContext ctx) { return new Function( ctx.nullNotnull().NOT() == null ? IS_NULL.getName().getFunctionName() : IS_NOT_NULL.getName().getFunctionName(), @@ -104,13 +116,28 @@ public UnresolvedExpression visitIsNullPredicate(OpenDistroSQLParser.IsNullPredi } @Override - public UnresolvedExpression visitLikePredicate(OpenDistroSQLParser.LikePredicateContext ctx) { + public UnresolvedExpression visitLikePredicate(LikePredicateContext ctx) { return new Function( ctx.NOT() == null ? LIKE.getName().getFunctionName() : NOT_LIKE.getName().getFunctionName(), Arrays.asList(visit(ctx.left), visit(ctx.right))); } + @Override + public UnresolvedExpression visitAndExpression(AndExpressionContext ctx) { + return new And(visit(ctx.left), visit(ctx.right)); + } + + @Override + public UnresolvedExpression visitOrExpression(OrExpressionContext ctx) { + return new Or(visit(ctx.left), visit(ctx.right)); + } + + @Override + public UnresolvedExpression visitNotExpression(NotExpressionContext ctx) { + return new Not(visit(ctx.expression())); + } + @Override public UnresolvedExpression visitString(StringContext ctx) { return AstDSL.stringLiteral(StringUtils.unquoteText(ctx.getText())); @@ -132,29 +159,29 @@ public UnresolvedExpression visitBoolean(BooleanContext ctx) { } @Override - public UnresolvedExpression visitNullLiteral(OpenDistroSQLParser.NullLiteralContext ctx) { + public UnresolvedExpression visitNullLiteral(NullLiteralContext ctx) { return AstDSL.nullLiteral(); } @Override - public UnresolvedExpression visitDateLiteral(OpenDistroSQLParser.DateLiteralContext ctx) { + public UnresolvedExpression visitDateLiteral(DateLiteralContext ctx) { return AstDSL.dateLiteral(StringUtils.unquoteText(ctx.date.getText())); } @Override - public UnresolvedExpression visitTimeLiteral(OpenDistroSQLParser.TimeLiteralContext ctx) { + public UnresolvedExpression visitTimeLiteral(TimeLiteralContext ctx) { return AstDSL.timeLiteral(StringUtils.unquoteText(ctx.time.getText())); } @Override public UnresolvedExpression visitTimestampLiteral( - OpenDistroSQLParser.TimestampLiteralContext ctx) { + TimestampLiteralContext ctx) { return AstDSL.timestampLiteral(StringUtils.unquoteText(ctx.timestamp.getText())); } @Override public UnresolvedExpression visitBinaryComparisonPredicate( - OpenDistroSQLParser.BinaryComparisonPredicateContext ctx) { + BinaryComparisonPredicateContext ctx) { String functionName = ctx.comparisonOperator().getText(); return new Function( functionName.equals("<>") ? "!=" : functionName, diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java index f8f9df9875..0f516de40d 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -91,10 +91,27 @@ public void canNotParseIndexNameSingleQuoted() { () -> parser.parse("SELECT * FROM 'test'")); } + @Test + public void canParseWhereClause() { + assertNotNull(parser.parse("SELECT name FROM test WHERE age = 10")); + } + + @Test + public void canParseSelectClauseWithLogicalOperator() { + assertNotNull(parser.parse( + "SELECT age = 10 AND name = 'John' OR NOT (balance > 1000) FROM test")); + } + + @Test + public void canParseWhereClauseWithLogicalOperator() { + assertNotNull(parser.parse("SELECT name FROM test " + + "WHERE age = 10 AND name = 'John' OR NOT (balance > 1000)")); + } + @Test public void canNotParseInvalidSelect() { assertThrows(SyntaxCheckException.class, - () -> parser.parse("SELECT * FROM test WHERE age = 10")); + () -> parser.parse("SELECT * FROM test WHERE age = 10 GROUP BY name")); } } \ No newline at end of file diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java index 3de6b9920b..dafb74e9c9 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java @@ -19,6 +19,7 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.alias; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.booleanLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.doubleLiteral; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.filter; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.function; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.intLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.project; @@ -145,6 +146,23 @@ public void can_build_select_fields_with_alias_quoted() { ); } + @Test + public void can_build_where_clause() { + assertEquals( + project( + filter( + relation("test"), + function( + "=", + qualifiedName("name"), + stringLiteral("John")) + ), + alias("name", qualifiedName("name")) + ), + buildAST("SELECT name FROM test WHERE name = 'John'") + ); + } + private UnresolvedPlan buildAST(String query) { ParseTree parseTree = parser.parse(query); return parseTree.accept(new AstBuilder(query)); diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java index cde9340313..a6e5d6f798 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -16,12 +16,15 @@ package com.amazon.opendistroforelasticsearch.sql.sql.parser; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.and; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.booleanLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.dateLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.doubleLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.function; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.intLiteral; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.not; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.nullLiteral; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.or; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.timeLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.timestampLiteral; @@ -195,6 +198,24 @@ public void canBuildLikeExpression() { ); } + @Test + public void canBuildLogicalExpression() { + assertEquals( + and(booleanLiteral(true), booleanLiteral(false)), + buildExprAst("true AND false") + ); + + assertEquals( + or(booleanLiteral(true), booleanLiteral(false)), + buildExprAst("true OR false") + ); + + assertEquals( + not(booleanLiteral(false)), + buildExprAst("NOT false") + ); + } + private Node buildExprAst(String expr) { OpenDistroSQLLexer lexer = new OpenDistroSQLLexer(new CaseInsensitiveCharStream(expr)); OpenDistroSQLParser parser = new OpenDistroSQLParser(new CommonTokenStream(lexer));