Address review comments, including reverting some changes.

Signed-off-by: currantw <[email protected]>
opensearch-project · Dec 19, 2024 · 953a1fc · 953a1fc
1 parent cec3f39
commit 953a1fc
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 8 deletions.
diff --git 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 '.+@(?<host>.+)' | stats count() by host`
 - `source=accounts | parse email '.+@(?<host>.+)' | eval eval_result=1 | fields host, eval_result`
 - `source=accounts | parse email '.+@(?<host>.+)' | where age > 45 | sort - age | fields age, email, host`
-- `source=accounts | parse address '(?<streetNumber>\d+) (?<street>.+)' | where streetNumber > 500 | sort num(streetNumber) | fields streetNumber, street`
+- `source=accounts | parse address '(?<streetNumber>\d+) (?<street>.+)' | 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
@@ -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 '(?<streetNumber>\d+) (?<street>.+)' | where cast(streetNumber as int) > 500 | sort num(streetNumber) | fields streetNumber, street ;
+    os> source=accounts | parse address '(?<streetNumber>\d+) (?<street>.+)' | 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
@@ -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<Unresol
                     .put("isnotnull", IS_NOT_NULL.getName().getFunctionName())
                     .put("ispresent", IS_NOT_NULL.getName().getFunctionName())
                     .build();
-    private final AstBuilder astBuilder;
+    private AstBuilder astBuilder;
 
     public AstExpressionBuilder(AstBuilder astBuilder) {
         this.astBuilder = astBuilder;
@@ -262,6 +265,7 @@ public UnresolvedExpression visitCaseExpr(OpenSearchPPLParser.CaseExprContext ct
     public UnresolvedExpression visitIsEmptyExpression(OpenSearchPPLParser.IsEmptyExpressionContext ctx) {
         Function trimFunction = new Function(TRIM.getName().getFunctionName(), Collections.singletonList(this.visitFunctionArg(ctx.functionArg())));
         Function lengthFunction = new Function(LENGTH.getName().getFunctionName(), Collections.singletonList(trimFunction));
+        Compare lengthEqualsZero = new Compare(EQUAL.getName().getFunctionName(), lengthFunction, new Literal(0, DataType.INTEGER));
         Literal whenCompareValue = new Literal(0, DataType.INTEGER);
         Literal isEmptyFalse = new Literal(false, DataType.BOOLEAN);
         Literal isEmptyTrue = new Literal(true, DataType.BOOLEAN);
@@ -450,10 +454,12 @@ public UnresolvedExpression visitLambda(OpenSearchPPLParser.LambdaContext ctx) {
 
     private List<UnresolvedExpression> timestampFunctionArguments(
             OpenSearchPPLParser.TimestampFunctionCallContext ctx) {
-        return Arrays.asList(
-                new Literal(ctx.timestampFunction().simpleDateTimePart().getText(), DataType.STRING),
-                visitFunctionArg(ctx.timestampFunction().firstArg),
-                visitFunctionArg(ctx.timestampFunction().secondArg));
+        List<UnresolvedExpression> 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<? extends ParserRuleContext> ctx) {

diff --git a/...rc/test/scala/org/opensearch/flint/spark/ppl/PPLLogicalPlanParseTranslatorTestSuite.scala b/...rc/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 '(?<streetNumber>\\d+) (?<street>.+)' | where streetNumber > 500 | sort num(streetNumber) | fields streetNumber, street"
+      "source=t | parse address '(?<streetNumber>\\d+) (?<street>.+)' | where streetNumber > 500 | sort cast(streetNumber as integer) | fields streetNumber, street"
 
     val logPlan = planTransformer.visit(plan(pplParser, query), context)