From 05d260b7b81f25880ecbedb40cfb334dc9159ca9 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 11 Jan 2023 16:06:19 -0800 Subject: [PATCH 1/3] Support constant in aggregation. Query sample: ```sql select 1 from calcs GROUP BY 1; ``` Signed-off-by: Yury-Fridlyand --- .../sql/legacy/AggregationExpressionIT.java | 25 ++++++++----------- .../dsl/AggregationBuilderHelper.java | 3 ++- .../dsl/BucketAggregationBuilderTest.java | 24 ++++++++++++++++++ 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java index 854a33fc91..cc99092e62 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java @@ -46,8 +46,6 @@ public void noGroupKeyMaxAddMinShouldPass() { verifyDataRows(response, rows(60)); } - // todo age field should has long type instead of integer type. - @Ignore @Test public void noGroupKeyMaxAddLiteralShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -59,7 +57,6 @@ public void noGroupKeyMaxAddLiteralShouldPass() { verifyDataRows(response, rows(41)); } - @Ignore("skip this test because the old engine returns an integer instead of a double type") @Test public void noGroupKeyAvgOnIntegerShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -103,8 +100,6 @@ public void hasGroupKeyMaxAddMinShouldPass() { rows("f", 60)); } - // todo age field should has long type instead of integer type. - @Ignore @Test public void hasGroupKeyMaxAddLiteralShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -121,7 +116,6 @@ public void hasGroupKeyMaxAddLiteralShouldPass() { rows("f", 1)); } - @Ignore("Handled by v2 engine which returns 'name': 'Log(MAX(age) + MIN(age))' instead") @Test public void noGroupKeyLogMaxAddMinShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -149,8 +143,6 @@ public void hasGroupKeyLogMaxAddMinShouldPass() { rows("f", 4.0943445622221d)); } - // todo age field should has long type instead of integer type. - @Ignore @Test public void AddLiteralOnGroupKeyShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -189,8 +181,6 @@ public void logWithAddLiteralOnGroupKeyShouldPass() { rows("m", 3.4339872044851463d, 49433)); } - // todo max field should has long as type instead of integer type. - @Ignore @Test public void logWithAddLiteralOnGroupKeyAndMaxSubtractLiteralShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -213,7 +203,6 @@ public void logWithAddLiteralOnGroupKeyAndMaxSubtractLiteralShouldPass() { /** * The date is in JDBC format. */ - @Ignore("skip this test due to inconsistency in type in new engine") @Test public void groupByDateShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -227,10 +216,9 @@ public void groupByDateShouldPass() { schema("birthdate", null, "date"), schema("count", "count", "integer")); verifyDataRows(response, - rows("2018-06-23 00:00:00.000", 1)); + rows("2018-06-23 00:00:00", 1)); } - @Ignore("skip this test due to inconsistency in type in new engine") @Test public void groupByDateWithAliasShouldPass() { JSONObject response = executeJdbcRequest(String.format( @@ -244,7 +232,7 @@ public void groupByDateWithAliasShouldPass() { schema("birth", "birth", "date"), schema("count", "count", "integer")); verifyDataRows(response, - rows("2018-06-23 00:00:00.000", 1)); + rows("2018-06-23 00:00:00", 1)); } @Test @@ -256,4 +244,13 @@ public void aggregateCastStatementShouldNotReturnZero() { verifySchema(response, schema("SUM(CAST(male AS INT))", "male_sum", "integer")); verifyDataRows(response, rows(4)); } + + @Test + public void groupByConstantShouldPass() { + JSONObject response = executeJdbcRequest(String.format( + "select 1 from %s GROUP BY 1", Index.BANK.getName())); + + verifySchema(response, schema("1", null, "integer")); + verifyDataRows(response, rows(1)); + } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java index b0a935f623..5b4ea09a97 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java @@ -15,6 +15,7 @@ import org.opensearch.script.Script; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.LiteralExpression; import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.opensearch.storage.script.ScriptUtils; import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer; @@ -38,7 +39,7 @@ public T build(Expression expression, Function fieldBuilder, if (expression instanceof ReferenceExpression) { String fieldName = ((ReferenceExpression) expression).getAttr(); return fieldBuilder.apply(ScriptUtils.convertTextToKeyword(fieldName, expression.type())); - } else if (expression instanceof FunctionExpression) { + } else if (expression instanceof FunctionExpression || expression instanceof LiteralExpression) { return scriptBuilder.apply(new Script( DEFAULT_SCRIPT_TYPE, EXPRESSION_LANG_NAME, serializer.serialize(expression), emptyMap())); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java index 9f21a145ed..c28b486768 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java @@ -11,6 +11,9 @@ import static org.opensearch.common.xcontent.ToXContent.EMPTY_PARAMS; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import static org.opensearch.sql.data.type.ExprCoreType.TIME; +import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; +import static org.opensearch.sql.expression.DSL.literal; import static org.opensearch.sql.expression.DSL.named; import static org.opensearch.sql.expression.DSL.ref; import static org.opensearch.sql.opensearch.data.type.OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD; @@ -68,6 +71,27 @@ void should_build_bucket_with_field() { asc(named("age", ref("age", INTEGER)))))); } + @Test + void should_build_bucket_with_literal() { + var literal = literal(1); + when(serializer.serialize(literal)).thenReturn("mock-serialize"); + assertEquals( + "{\n" + + " \"terms\" : {\n" + + " \"script\" : {\n" + + " \"source\" : \"mock-serialize\",\n" + + " \"lang\" : \"opensearch_query_expression\"\n" + + " },\n" + + " \"missing_bucket\" : true,\n" + + " \"missing_order\" : \"first\",\n" + + " \"order\" : \"asc\"\n" + + " }\n" + + "}", + buildQuery( + Arrays.asList( + asc(named(literal))))); + } + @Test void should_build_bucket_with_keyword_field() { assertEquals( From 7b0ed045339d2d45001489ab256aa2cf78290766 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 11 Jan 2023 18:59:28 -0800 Subject: [PATCH 2/3] Fix tests. Signed-off-by: Yury-Fridlyand --- .../sql/legacy/AggregationExpressionIT.java | 46 +++++++++---------- .../dsl/AggregationBuilderHelper.java | 3 +- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java index cc99092e62..af6e2ad492 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java @@ -49,29 +49,29 @@ public void noGroupKeyMaxAddMinShouldPass() { @Test public void noGroupKeyMaxAddLiteralShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT MAX(age) + 1 as add " + + "SELECT MAX(age) + 1 as `add` " + "FROM %s", Index.ACCOUNT.getName())); - verifySchema(response, schema("add", "add", "long")); + verifySchema(response, schema("MAX(age) + 1", "add", "long")); verifyDataRows(response, rows(41)); } @Test public void noGroupKeyAvgOnIntegerShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT AVG(age) as avg " + + "SELECT AVG(age) as `avg` " + "FROM %s", Index.BANK.getName())); - verifySchema(response, schema("avg", "avg", "double")); - verifyDataRows(response, rows(34)); + verifySchema(response, schema("AVG(age)", "avg", "double")); + verifyDataRows(response, rows(34D)); } @Test public void hasGroupKeyAvgOnIntegerShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT gender, AVG(age) as avg " + + "SELECT gender, AVG(age) as `avg` " + "FROM %s " + "GROUP BY gender", Index.BANK.getName())); @@ -103,27 +103,27 @@ public void hasGroupKeyMaxAddMinShouldPass() { @Test public void hasGroupKeyMaxAddLiteralShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT gender, MAX(age) + 1 as add " + + "SELECT gender, MAX(age) + 1 as `add` " + "FROM %s " + "GROUP BY gender", Index.ACCOUNT.getName())); verifySchema(response, schema("gender", null, "text"), - schema("add", "add", "long")); + schema("MAX(age) + 1", "add", "long")); verifyDataRows(response, - rows("m", 1), - rows("f", 1)); + rows("m", 41), + rows("f", 41)); } @Test public void noGroupKeyLogMaxAddMinShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT Log(MAX(age) + MIN(age)) as log " + + "SELECT Log(MAX(age) + MIN(age)) as `log` " + "FROM %s", Index.ACCOUNT.getName())); - verifySchema(response, schema("log", "log", "double")); + verifySchema(response, schema("Log(MAX(age) + MIN(age))", "log", "double")); verifyDataRows(response, rows(4.0943445622221d)); } @@ -146,7 +146,7 @@ public void hasGroupKeyLogMaxAddMinShouldPass() { @Test public void AddLiteralOnGroupKeyShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT gender, age+10, max(balance) as max " + + "SELECT gender, age+10, max(balance) as `max` " + "FROM %s " + "WHERE gender = 'm' and age < 22 " + "GROUP BY gender, age " + @@ -155,8 +155,8 @@ public void AddLiteralOnGroupKeyShouldPass() { verifySchema(response, schema("gender", null, "text"), - schema("age", "age", "long"), - schema("max", "max", "long")); + schema("age+10", null, "long"), + schema("max(balance)", "max", "long")); verifyDataRows(response, rows("m", 30, 49568), rows("m", 31, 49433)); @@ -193,8 +193,8 @@ public void logWithAddLiteralOnGroupKeyAndMaxSubtractLiteralShouldPass() { verifySchema(response, schema("gender", null, "text"), - schema("logAge", "logAge", "double"), - schema("max", "max", "long")); + schema("Log(age+10)", "logAge", "double"), + schema("max(balance) - 100", "max", "long")); verifyDataRows(response, rows("m", 3.4011973816621555d, 49468), rows("m", 3.4339872044851463d, 49333)); @@ -206,15 +206,15 @@ public void logWithAddLiteralOnGroupKeyAndMaxSubtractLiteralShouldPass() { @Test public void groupByDateShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT birthdate, count(*) as count " + + "SELECT birthdate, count(*) as `count` " + "FROM %s " + "WHERE age < 30 " + "GROUP BY birthdate ", Index.BANK.getName())); verifySchema(response, - schema("birthdate", null, "date"), - schema("count", "count", "integer")); + schema("birthdate", null, "timestamp"), + schema("count(*)", "count", "integer")); verifyDataRows(response, rows("2018-06-23 00:00:00", 1)); } @@ -222,15 +222,15 @@ public void groupByDateShouldPass() { @Test public void groupByDateWithAliasShouldPass() { JSONObject response = executeJdbcRequest(String.format( - "SELECT birthdate as birth, count(*) as count " + + "SELECT birthdate as birth, count(*) as `count` " + "FROM %s " + "WHERE age < 30 " + "GROUP BY birthdate ", Index.BANK.getName())); verifySchema(response, - schema("birth", "birth", "date"), - schema("count", "count", "integer")); + schema("birthdate", "birth", "timestamp"), + schema("count(*)", "count", "integer")); verifyDataRows(response, rows("2018-06-23 00:00:00", 1)); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java index 5b4ea09a97..001d7af970 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/AggregationBuilderHelper.java @@ -39,7 +39,8 @@ public T build(Expression expression, Function fieldBuilder, if (expression instanceof ReferenceExpression) { String fieldName = ((ReferenceExpression) expression).getAttr(); return fieldBuilder.apply(ScriptUtils.convertTextToKeyword(fieldName, expression.type())); - } else if (expression instanceof FunctionExpression || expression instanceof LiteralExpression) { + } else if (expression instanceof FunctionExpression + || expression instanceof LiteralExpression) { return scriptBuilder.apply(new Script( DEFAULT_SCRIPT_TYPE, EXPRESSION_LANG_NAME, serializer.serialize(expression), emptyMap())); From 47dbb2f86746a014fbd6ef5bacce852bce7151ac Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 12 Jan 2023 11:29:28 -0800 Subject: [PATCH 3/3] Remove unused imports. Signed-off-by: Yury-Fridlyand --- .../script/aggregation/dsl/BucketAggregationBuilderTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java index c28b486768..25fee2047a 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/BucketAggregationBuilderTest.java @@ -11,8 +11,6 @@ import static org.opensearch.common.xcontent.ToXContent.EMPTY_PARAMS; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import static org.opensearch.sql.data.type.ExprCoreType.TIME; -import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; import static org.opensearch.sql.expression.DSL.literal; import static org.opensearch.sql.expression.DSL.named; import static org.opensearch.sql.expression.DSL.ref;