Skip to content

Add asc keyword to sort command #4113

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

Merged
Swiddis merged 8 commits into from
Sep 2, 2025
Merged

Add asc keyword to sort command #4113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6a161fd
add asc keyword to sort
ritvibhatt Aug 22, 2025
1c27280
add tests and update docs
ritvibhatt Aug 22, 2025
96cda7d
remove a
ritvibhatt Aug 22, 2025
9f5eae5
add a keyword
ritvibhatt Aug 26, 2025
7dcd9cf
update doc
ritvibhatt Aug 26, 2025
617090c
empty
ritvibhatt Aug 27, 2025
787621b
fix formatting
ritvibhatt Aug 27, 2025
6c54b41
empty
ritvibhatt Aug 27, 2025
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
6 changes: 3 additions & 3 deletions docs/user/ppl/cmd/sort.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ Description

Syntax
============
sort [count] <[+|-] sort-field>... [desc|d]
sort [count] <[+|-] sort-field>... [asc|a|desc|d]


* count: optional. The number of results to return. **Default:** returns all results. Specifying a count of 0 or less than 0 also returns all results.
* count (Since 3.3): optional. The number of results to return. **Default:** returns all results. Specifying a count of 0 or less than 0 also returns all results.
Copy link
Collaborator

@RyanL1997 RyanL1997 Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have these covered in example section?

Copy link
Contributor Author

@ritvibhatt ritvibhatt Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! just added 'Since 3.3' in this PR but added examples in previous PR

* [+|-]: optional. The plus [+] stands for ascending order and NULL/MISSING first and a minus [-] stands for descending order and NULL/MISSING last. **Default:** ascending order and NULL/MISSING first.
* sort-field: mandatory. The field used to sort. Can use ``auto(field)``, ``str(field)``, ``ip(field)``, or ``num(field)`` to specify how to interpret field values.
* [desc|d]: optional. Reverses the sort results. If multiple fields are specified, reverses order of the first field then for all duplicate values of the first field, reverses the order of the values of the second field and so on.
* [asc|a|desc|d] (Since 3.3): optional. asc/a keeps the sort order as specified. desc/d reverses the sort results. If multiple fields are specified with desc/d, reverses order of the first field then for all duplicate values of the first field, reverses the order of the values of the second field and so on. **Default:** asc.
Copy link
Collaborator

@RyanL1997 RyanL1997 Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Consider rewording to 'sorts in ascending order' for clarity.

Copy link
Contributor Author

@ritvibhatt ritvibhatt Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior for asc is it will keep whatever the specified field sort order was, so for sort firstname, -lastname asc firstname will still be sorted ascending and lastname will still be sorted descending. Don't know if 'sorts in ascending order' would make that behavior clear



Example 1: Sort by one field
Expand Down
96 changes: 96 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLSortIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,4 +234,100 @@ public void testSortDate() throws IOException {
rows("Virginia", "2018-08-19 00:00:00"),
rows("Dale", "2018-11-13 23:33:20"));
}

@Test
public void testSortWithAscKeyword() throws IOException {
Copy link
Collaborator

@RyanL1997 RyanL1997 Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Consider adding negative test cases:

  • Invalid combinations like 'sort field asc desc'
  • Edge cases with count=0"

JSONObject result =
executeQuery(
String.format(
"source=%s | sort account_number asc | fields account_number, firstname",
TEST_INDEX_BANK));
verifySchema(result, schema("account_number", "bigint"), schema("firstname", "string"));
verifyDataRowsInOrder(
result,
rows(1, "Amber JOHnny"),
rows(6, "Hattie"),
rows(13, "Nanette"),
rows(18, "Dale"),
rows(20, "Elinor"),
rows(25, "Virginia"),
rows(32, "Dillard"));
}

@Test
public void testSortWithAKeyword() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | sort account_number a | fields account_number, firstname",
TEST_INDEX_BANK));
verifySchema(result, schema("account_number", "bigint"), schema("firstname", "string"));
verifyDataRowsInOrder(
result,
rows(1, "Amber JOHnny"),
rows(6, "Hattie"),
rows(13, "Nanette"),
rows(18, "Dale"),
rows(20, "Elinor"),
rows(25, "Virginia"),
rows(32, "Dillard"));
}

@Test
public void testSortWithDescKeyword() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | sort account_number desc | fields account_number, firstname",
TEST_INDEX_BANK));
verifySchema(result, schema("account_number", "bigint"), schema("firstname", "string"));
verifyDataRowsInOrder(
result,
rows(32, "Dillard"),
rows(25, "Virginia"),
rows(20, "Elinor"),
rows(18, "Dale"),
rows(13, "Nanette"),
rows(6, "Hattie"),
rows(1, "Amber JOHnny"));
}

@Test
public void testSortWithCount() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | sort 3 account_number | fields account_number, firstname",
TEST_INDEX_BANK));
verifySchema(result, schema("account_number", "bigint"), schema("firstname", "string"));
verifyDataRowsInOrder(result, rows(1, "Amber JOHnny"), rows(6, "Hattie"), rows(13, "Nanette"));
}

@Test
public void testSortWithStrCast() throws IOException {

JSONObject result =
executeQuery(
String.format(
"source=%s | sort STR(account_number) | fields account_number", TEST_INDEX_BANK));
verifyDataRowsInOrder(
result, rows(1), rows(13), rows(18), rows(20), rows(25), rows(32), rows(6));
}

@Test
public void testSortWithAutoCast() throws IOException {
JSONObject result =
executeQuery(
String.format("source=%s | sort AUTO(age) | fields firstname, age", TEST_INDEX_BANK));
verifySchema(result, schema("firstname", "string"), schema("age", "int"));
verifyDataRowsInOrder(
result,
rows("Nanette", 28),
rows("Amber JOHnny", 32),
rows("Dale", 33),
rows("Dillard", 34),
rows("Hattie", 36),
rows("Elinor", 36),
rows("Virginia", 39));
}
}
36 changes: 36 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/ppl/SortCommandIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,40 @@ public void testSortWithNumCast() throws IOException {
verifyOrder(
result, rows("1234"), rows("3985"), rows("4085"), rows("4321"), rows("6245"), rows("9876"));
}

@Test
public void testSortWithAsc() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | sort account_number asc | fields account_number", TEST_INDEX_BANK));
verifyOrder(result, rows(1), rows(6), rows(13), rows(18), rows(20), rows(25), rows(32));
}

@Test
public void testSortWithA() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | sort account_number a | fields account_number", TEST_INDEX_BANK));
verifyOrder(result, rows(1), rows(6), rows(13), rows(18), rows(20), rows(25), rows(32));
}

@Test
public void testSortWithAscMultipleFields() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | sort age, account_number asc | fields age, account_number",
TEST_INDEX_BANK));
verifyOrder(
result,
rows(28, 13),
rows(32, 1),
rows(33, 18),
rows(34, 32),
rows(36, 6),
rows(36, 20),
rows(39, 25));
}
}
2 changes: 2 additions & 0 deletions ppl/src/main/antlr/OpenSearchPPLLexer.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ AS: 'AS';
BY: 'BY';
SOURCE: 'SOURCE';
INDEX: 'INDEX';
A: 'A';
ASC: 'ASC';
D: 'D';
DESC: 'DESC';
DATASOURCES: 'DATASOURCES';
Expand Down
4 changes: 3 additions & 1 deletion ppl/src/main/antlr/OpenSearchPPLParser.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ dedupCommand
;

sortCommand
: SORT (count = integerLiteral)? sortbyClause (DESC | D)?
: SORT (count = integerLiteral)? sortbyClause (ASC | A | DESC | D)?
Copy link
Collaborator

@RyanL1997 RyanL1997 Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider the precedence if someone accidentally specifies both asc and desc? The grammar allows it but behavior might be undefined.

Copy link
Contributor Author

@ritvibhatt ritvibhatt Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked using both asc and desc will result in an error

Copy link
Collaborator

@RyanL1997 RyanL1997 Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. Thanks for confirming that.

;

reverseCommand
Expand Down Expand Up @@ -1201,6 +1201,8 @@ keywordsCanBeId
| EXISTS
| SOURCE
| INDEX
| A
| ASC
| DESC
| DATASOURCES
| FROM
Expand Down
36 changes: 36 additions & 0 deletions ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,42 @@ public void testSortCommandWithMultipleFieldsAndDesc() {
exprList(argument("asc", booleanLiteral(true)), argument("type", nullLiteral())))));
}

@Test
public void testSortCommandWithAsc() {
assertEqual(
"source=t | sort f1 asc",
sort(
relation("t"),
field(
"f1",
exprList(argument("asc", booleanLiteral(true)), argument("type", nullLiteral())))));
}

@Test
public void testSortCommandWithA() {
assertEqual(
"source=t | sort f1 a",
sort(
relation("t"),
field(
"f1",
exprList(argument("asc", booleanLiteral(true)), argument("type", nullLiteral())))));
}

@Test
public void testSortCommandWithMultipleFieldsAndAsc() {
assertEqual(
"source=t | sort f1, f2 asc",
sort(
relation("t"),
field(
"f1",
exprList(argument("asc", booleanLiteral(true)), argument("type", nullLiteral()))),
field(
"f2",
exprList(argument("asc", booleanLiteral(true)), argument("type", nullLiteral())))));
}

@Test
public void testEvalCommand() {
assertEqual(
Expand Down
Loading