From 3ee34911797c496f87fbd69d15af0d87115670bc Mon Sep 17 00:00:00 2001 From: forestmvey Date: Fri, 29 Jul 2022 11:50:54 -0700 Subject: [PATCH] Code cleanup, adding parsing failure tests, and adding tests for highlight acceptance as a string literal as well as qualified name. Signed-off-by: forestmvey --- .../function/OpenSearchFunctions.java | 1 + .../physical/HighlightOperatorTest.java | 2 +- .../sql/sql/HighlightFunctionIT.java | 9 ++++--- .../response/OpenSearchResponseTest.java | 2 +- .../sql/sql/antlr/HighlightTest.java | 13 ++++++++++ .../sql/sql/parser/AstBuilderTest.java | 25 +++++++++++++------ 6 files changed, 40 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index f7d922cd9b..c3e5cc5594 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -15,6 +15,7 @@ import java.util.Map; import java.util.stream.Collectors; import lombok.experimental.UtilityClass; +import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/HighlightOperatorTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/HighlightOperatorTest.java index 349820dada..55745d51f4 100644 --- a/core/src/test/java/org/opensearch/sql/planner/physical/HighlightOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/planner/physical/HighlightOperatorTest.java @@ -20,7 +20,7 @@ public class HighlightOperatorTest extends PhysicalPlanTestBase { @Test - public void highlight_all_test() { + public void valid_highlight_operator_test() { PhysicalPlan plan = new HighlightOperator(new TestScan(), DSL.ref("*", STRING)); List result = execute(plan); assertEquals(5, result.size()); diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/HighlightFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/HighlightFunctionIT.java index 871285f454..1c171d7bb1 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/HighlightFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/HighlightFunctionIT.java @@ -24,7 +24,8 @@ protected void init() throws Exception { public void single_highlight_test() { String query = "SELECT Tags, highlight('Tags') FROM %s WHERE match(Tags, 'yeast') LIMIT 1"; JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_BEER)); - verifySchema(response, schema("Tags", null, "text"), schema("highlight('Tags')", null, "keyword")); + verifySchema(response, schema("Tags", null, "text"), + schema("highlight('Tags')", null, "keyword")); assertEquals(1, response.getInt("total")); } @@ -32,7 +33,8 @@ public void single_highlight_test() { public void accepts_unquoted_test() { String query = "SELECT Tags, highlight(Tags) FROM %s WHERE match(Tags, 'yeast') LIMIT 1"; JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_BEER)); - verifySchema(response, schema("Tags", null, "text"), schema("highlight(Tags)", null, "keyword")); + verifySchema(response, schema("Tags", null, "text"), + schema("highlight(Tags)", null, "keyword")); assertEquals(1, response.getInt("total")); } @@ -40,7 +42,8 @@ public void accepts_unquoted_test() { public void multiple_highlight_test() { String query = "SELECT highlight(Title), highlight(Body) FROM %s WHERE MULTI_MATCH([Title, Body], 'hops') LIMIT 1"; JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_BEER)); - verifySchema(response, schema("highlight(Title)", null, "keyword"), schema("highlight(Body)", null, "keyword")); + verifySchema(response, schema("highlight(Title)", null, "keyword"), + schema("highlight(Body)", null, "keyword")); assertEquals(1, response.getInt("total")); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java index ba244bacc6..2ad41c7a63 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java @@ -183,4 +183,4 @@ void highlight_iterator() { assertTrue(expected.equals(result)); } } -} \ No newline at end of file +} diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/HighlightTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/HighlightTest.java index d88b965368..6826a37c0b 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/HighlightTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/HighlightTest.java @@ -27,6 +27,19 @@ void wildcard_test() { @Test void highlight_all_test() { + acceptQuery("SELECT HIGHLIGHT('*') FROM Index WHERE MULTI_MATCH([Tags, Body], 'Time')"); } + + @Test + void multiple_parameters_failure_test() { + rejectQuery("SELECT HIGHLIGHT(Tags1, Tags2) FROM Index " + + "WHERE MULTI_MATCH([Tags, Body], 'Time')"); + } + + @Test + void no_parameters_failure_test() { + rejectQuery("SELECT HIGHLIGHT() FROM Index " + + "WHERE MULTI_MATCH([Tags, Body], 'Time')"); + } } diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java index a2a27bb2df..9f2f8b5f92 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java @@ -616,12 +616,14 @@ public void describe_and_column_compatible_with_old_engine_syntax() { @Test public void can_build_alias_by_keywords() { + var expected = project( + relation("test"), + alias("avg_age", qualifiedName("avg_age"), "avg") + ); + var comp = buildAST("SELECT avg_age AS avg FROM test"); assertEquals( - project( - relation("test"), - alias("avg_age", qualifiedName("avg_age"), "avg") - ), - buildAST("SELECT avg_age AS avg FROM test") + expected, + comp ); } @@ -670,14 +672,23 @@ public void can_build_limit_clause_with_offset() { } @Test - public void can_build_highlight() { + public void can_build_qualified_name_highlight() { assertEquals( project(relation("test"), - alias("highlight(fieldA)", highlight(AstDSL.stringLiteral("fieldA")))), + alias("highlight(fieldA)", highlight(AstDSL.qualifiedName("fieldA")))), buildAST("SELECT highlight(fieldA) FROM test") ); } + @Test + public void can_build_string_literal_highlight() { + assertEquals( + project(relation("test"), + alias("highlight(\"fieldA\")", highlight(AstDSL.stringLiteral("fieldA")))), + buildAST("SELECT highlight(\"fieldA\") FROM test") + ); + } + private UnresolvedPlan buildAST(String query) { ParseTree parseTree = parser.parse(query); return parseTree.accept(new AstBuilder(query));