Skip to content

Refactor PPL sort desc/asc logic #5224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ritvibhatt wants to merge 3 commits into
base: main
Choose a base branch
from
+18 −62
Open

Refactor PPL sort desc/asc logic #5224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a606f60
refactor ppl sort logic
ritvibhatt Mar 10, 2026
cf133f4
apply spotless
ritvibhatt Mar 10, 2026
f600d3b
remove unused code
ritvibhatt Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,18 +288,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
Expand All @@ -322,8 +324,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) {
Expand All @@ -334,7 +335,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
Expand Down
62 changes: 6 additions & 56 deletions ppl/src/main/java/org/opensearch/sql/ppl/utils/ArgumentFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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) {
Expand Down
Loading