From 953a1fc2db7324e0bece4e18c7d46234d5661f94 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 19 Dec 2024 13:00:51 -0800 Subject: [PATCH] Address review comments, including reverting some changes. Signed-off-by: currantw --- docs/ppl-lang/PPL-Example-Commands.md | 2 +- docs/ppl-lang/ppl-parse-command.md | 2 +- .../sql/ppl/parser/AstExpressionBuilder.java | 16 +++++++++++----- .../PPLLogicalPlanParseTranslatorTestSuite.scala | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/ppl-lang/PPL-Example-Commands.md b/docs/ppl-lang/PPL-Example-Commands.md index 1c95e7722..da938571a 100644 --- a/docs/ppl-lang/PPL-Example-Commands.md +++ b/docs/ppl-lang/PPL-Example-Commands.md @@ -274,7 +274,7 @@ source = table | where ispresent(a) | - `source=accounts | parse email '.+@(?.+)' | stats count() by host` - `source=accounts | parse email '.+@(?.+)' | eval eval_result=1 | fields host, eval_result` - `source=accounts | parse email '.+@(?.+)' | where age > 45 | sort - age | fields age, email, host` -- `source=accounts | parse address '(?\d+) (?.+)' | where streetNumber > 500 | sort num(streetNumber) | fields streetNumber, street` +- `source=accounts | parse address '(?\d+) (?.+)' | where streetNumber > 500 | sort cast(streetNumber as integer) | fields streetNumber, street` - **Note**: The `sort num` syntax is deprecated. To sort numerically, cast to a numerical data type - e.g. `sort cast(streetNumber as integer)`. See [#963](https://github.com/opensearch-project/opensearch-spark/issues/963) for more details. - Limitation: [see limitations](ppl-parse-command.md#limitations) diff --git a/docs/ppl-lang/ppl-parse-command.md b/docs/ppl-lang/ppl-parse-command.md index 6100564e7..66646503f 100644 --- a/docs/ppl-lang/ppl-parse-command.md +++ b/docs/ppl-lang/ppl-parse-command.md @@ -58,7 +58,7 @@ The example shows how to sort street numbers that are higher than 500 in ``addre PPL query: - os> source=accounts | parse address '(?\d+) (?.+)' | where cast(streetNumber as int) > 500 | sort num(streetNumber) | fields streetNumber, street ; + os> source=accounts | parse address '(?\d+) (?.+)' | where cast(streetNumber as int) > 500 | sort cast(streetNumber as integer) | fields streetNumber, street ; fetched rows / total rows = 3/3 +----------------+----------------+ | streetNumber | street | diff --git a/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index edc6a7923..50aea7234 100644 --- a/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -45,6 +45,8 @@ import org.opensearch.sql.ast.expression.subquery.ExistsSubquery; import org.opensearch.sql.ast.expression.subquery.InSubquery; import org.opensearch.sql.ast.expression.subquery.ScalarSubquery; +import org.opensearch.sql.ast.tree.Trendline; +import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.ppl.utils.ArgumentFactory; @@ -56,6 +58,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.EQUAL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.LENGTH; @@ -77,7 +80,7 @@ public class AstExpressionBuilder extends OpenSearchPPLParserBaseVisitor timestampFunctionArguments( OpenSearchPPLParser.TimestampFunctionCallContext ctx) { - return Arrays.asList( - new Literal(ctx.timestampFunction().simpleDateTimePart().getText(), DataType.STRING), - visitFunctionArg(ctx.timestampFunction().firstArg), - visitFunctionArg(ctx.timestampFunction().secondArg)); + List args = + Arrays.asList( + new Literal(ctx.timestampFunction().simpleDateTimePart().getText(), DataType.STRING), + visitFunctionArg(ctx.timestampFunction().firstArg), + visitFunctionArg(ctx.timestampFunction().secondArg)); + return args; } private QualifiedName visitIdentifiers(List ctx) { diff --git a/ppl-spark-integration/src/test/scala/org/opensearch/flint/spark/ppl/PPLLogicalPlanParseTranslatorTestSuite.scala b/ppl-spark-integration/src/test/scala/org/opensearch/flint/spark/ppl/PPLLogicalPlanParseTranslatorTestSuite.scala index 0fe0db6da..eadd550a3 100644 --- a/ppl-spark-integration/src/test/scala/org/opensearch/flint/spark/ppl/PPLLogicalPlanParseTranslatorTestSuite.scala +++ b/ppl-spark-integration/src/test/scala/org/opensearch/flint/spark/ppl/PPLLogicalPlanParseTranslatorTestSuite.scala @@ -125,7 +125,7 @@ class PPLLogicalPlanParseTranslatorTestSuite // TODO #963: Remove unimplemented sort syntax val query = - "source=t | parse address '(?\\d+) (?.+)' | where streetNumber > 500 | sort num(streetNumber) | fields streetNumber, street" + "source=t | parse address '(?\\d+) (?.+)' | where streetNumber > 500 | sort cast(streetNumber as integer) | fields streetNumber, street" val logPlan = planTransformer.visit(plan(pplParser, query), context)