Refactor PPL sort desc/asc logic (#5224)

Author: ritvibhatt

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java

@@ -300,18 +300,20 @@ public UnresolvedExpression visitRenameFieldExpression(RenameFieldExpressionCont
 
   @Override
   public UnresolvedExpression visitPrefixSortField(OpenSearchPPLParser.PrefixSortFieldContext ctx) {
-    return buildSortField(ctx.sortFieldExpression(), ctx);
+    boolean ascending = ctx.MINUS() == null;
+    return buildSortField(ctx.sortFieldExpression(), ascending);
   }
 
   @Override
   public UnresolvedExpression visitSuffixSortField(OpenSearchPPLParser.SuffixSortFieldContext ctx) {
-    return buildSortField(ctx.sortFieldExpression(), ctx);
+    boolean ascending = (ctx.DESC() == null && ctx.D() == null);
+    return buildSortField(ctx.sortFieldExpression(), ascending);
   }
 
   @Override
   public UnresolvedExpression visitDefaultSortField(
       OpenSearchPPLParser.DefaultSortFieldContext ctx) {
-    return buildSortField(ctx.sortFieldExpression(), ctx);
+    return buildSortField(ctx.sortFieldExpression(), true);
   }
 
   @Override
@@ -334,8 +336,7 @@ public UnresolvedExpression visitInvalidMixedSortField(
   }
 
   private Field buildSortField(
-      OpenSearchPPLParser.SortFieldExpressionContext sortFieldExpr,
-      OpenSearchPPLParser.SortFieldContext parentCtx) {
+      OpenSearchPPLParser.SortFieldExpressionContext sortFieldExpr, boolean ascending) {
     UnresolvedExpression fieldExpression = visit(sortFieldExpr.fieldExpression().qualifiedName());
 
     if (sortFieldExpr.IP() != null) {
@@ -346,7 +347,12 @@ private Field buildSortField(
       fieldExpression = new Cast(fieldExpression, AstDSL.stringLiteral("string"));
     }
     // AUTO() case uses the field expression as-is
-    return new Field(fieldExpression, ArgumentFactory.getArgumentList(parentCtx));
+
+    List<Argument> arguments =
+        Arrays.asList(
+            ArgumentFactory.createSortDirectionArgument(ascending),
+            ArgumentFactory.getTypeArgument(sortFieldExpr));
+    return new Field(fieldExpression, arguments);
   }
 
   @Override

ppl/src/main/java/org/opensearch/sql/ppl/utils/ArgumentFactory.java

@@ -27,14 +27,10 @@
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ChartCommandContext;
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DecimalLiteralContext;
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DedupCommandContext;
-import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DefaultSortFieldContext;
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.EventstatsCommandContext;
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.FieldsCommandContext;
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IntegerLiteralContext;
-import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.PrefixSortFieldContext;
-import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SortFieldContext;
 import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.StreamstatsCommandContext;
-import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SuffixSortFieldContext;
 import org.opensearch.sql.ppl.parser.AstExpressionBuilder;
 
 /** Util class to get all arguments as a list from the PPL command. */
@@ -155,63 +151,17 @@ public static List<Argument> getArgumentList(DedupCommandContext ctx) {
   }
 
   /**
-   * Get list of {@link Argument}.
+   * Creates an "asc" argument for sort field direction.
    *
-   * @param ctx SortFieldContext instance
-   * @return the list of arguments fetched from the sort field in sort command
+   * @param ascending true for ascending sort, false for descending
+   * @return Argument representing the sort direction
    */
-  public static List<Argument> getArgumentList(SortFieldContext ctx) {
-    if (ctx instanceof PrefixSortFieldContext) {
-      return getArgumentList((PrefixSortFieldContext) ctx);
-    } else if (ctx instanceof SuffixSortFieldContext) {
-      return getArgumentList((SuffixSortFieldContext) ctx);
-    } else {
-      return getArgumentList((DefaultSortFieldContext) ctx);
-    }
-  }
-
-  /**
-   * Get list of {@link Argument} for prefix sort field (+/- syntax).
-   *
-   * @param ctx PrefixSortFieldContext instance
-   * @return the list of arguments fetched from the prefix sort field
-   */
-  public static List<Argument> getArgumentList(PrefixSortFieldContext ctx) {
-    return Arrays.asList(
-        ctx.MINUS() != null
-            ? new Argument("asc", new Literal(false, DataType.BOOLEAN))
-            : new Argument("asc", new Literal(true, DataType.BOOLEAN)),
-        getTypeArgument(ctx.sortFieldExpression()));
-  }
-
-  /**
-   * Get list of {@link Argument} for suffix sort field (asc/desc syntax).
-   *
-   * @param ctx SuffixSortFieldContext instance
-   * @return the list of arguments fetched from the suffix sort field
-   */
-  public static List<Argument> getArgumentList(SuffixSortFieldContext ctx) {
-    return Arrays.asList(
-        (ctx.DESC() != null || ctx.D() != null)
-            ? new Argument("asc", new Literal(false, DataType.BOOLEAN))
-            : new Argument("asc", new Literal(true, DataType.BOOLEAN)),
-        getTypeArgument(ctx.sortFieldExpression()));
-  }
-
-  /**
-   * Get list of {@link Argument} for default sort field (no direction specified).
-   *
-   * @param ctx DefaultSortFieldContext instance
-   * @return the list of arguments fetched from the default sort field
-   */
-  public static List<Argument> getArgumentList(DefaultSortFieldContext ctx) {
-    return Arrays.asList(
-        new Argument("asc", new Literal(true, DataType.BOOLEAN)),
-        getTypeArgument(ctx.sortFieldExpression()));
+  public static Argument createSortDirectionArgument(boolean ascending) {
+    return new Argument("asc", new Literal(ascending, DataType.BOOLEAN));
   }
 
   /** Helper method to get type argument from sortFieldExpression. */
-  private static Argument getTypeArgument(OpenSearchPPLParser.SortFieldExpressionContext ctx) {
+  public static Argument getTypeArgument(OpenSearchPPLParser.SortFieldExpressionContext ctx) {
     if (ctx.AUTO() != null) {
       return new Argument("type", new Literal("auto", DataType.STRING));
     } else if (ctx.IP() != null) {