diff --git a/docs/user/ppl/cmd/head.rst b/docs/user/ppl/cmd/head.rst index c13a495a77f..c473ce3f455 100644 --- a/docs/user/ppl/cmd/head.rst +++ b/docs/user/ppl/cmd/head.rst @@ -8,6 +8,9 @@ head :local: :depth: 2 +.. versionadded:: 1.0.0 +.. versionchanged:: 3.3.0 + Added support for the `limit=` argument name. Description ============ @@ -16,9 +19,10 @@ Description Syntax ============ -head [] [from ] +head [limit=][] [from ] * : optional integer. number of results to return. **Default:** 10 + * For readability or convenience, this can be prefixed by `limit=`. * : integer after optional ``from``. number of results to skip. **Default:** 0 Example 1: Get first 10 results diff --git a/docs/user/ppl/cmd/top.rst b/docs/user/ppl/cmd/top.rst index a786d7ed9a9..f71317647d7 100644 --- a/docs/user/ppl/cmd/top.rst +++ b/docs/user/ppl/cmd/top.rst @@ -8,6 +8,9 @@ top :local: :depth: 2 +.. versionadded:: 1.0.0 +.. versionchanged:: 3.3.0 + Added support for the `limit=` argument name. Description =========== @@ -18,9 +21,10 @@ Syntax ====== top [N] [by-clause] -top [N] [top-options] [by-clause] ``(available from 3.1.0+)`` +top [limit=][N] [top-options] [by-clause] ``(available from 3.1.0+)`` * N: number of results to return. **Default**: 10 + * For readability or convenience, this can be prefixed by `limit=`. * field-list: mandatory. comma-delimited list of field names. * by-clause: optional. one or more fields to group the results by. * top-options: optional. options for the top command. Supported syntax is [countfield=] [showcount=]. @@ -89,7 +93,6 @@ PPL query:: | gender | count | |--------+-------| | M | 3 | - | F | 1 | +--------+-------+ diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index f103c52759a..70b1efa44bb 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -277,7 +277,7 @@ evalCommand ; headCommand - : HEAD (number = integerLiteral)? (FROM from = integerLiteral)? + : HEAD ((LIMIT EQUAL)? number = integerLiteral)? (FROM from = integerLiteral)? ; binCommand @@ -309,7 +309,7 @@ logSpanValue ; rareTopCommand - : (TOP | RARE) (number = integerLiteral)? rareTopOption* fieldList (byClause)? + : (TOP | RARE) ((LIMIT EQUAL)? number = integerLiteral)? rareTopOption* fieldList (byClause)? ; rareTopOption @@ -1431,6 +1431,7 @@ searchableKeyWord | FREQUENCY_THRESHOLD_PERCENTAGE | MAX_SAMPLE_COUNT | BUFFER_LIMIT + | LIMIT | SHOW_NUMBERED_TOKEN | WITH | REGEX diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/AstPlanningTestBase.java b/ppl/src/test/java/org/opensearch/sql/ppl/AstPlanningTestBase.java new file mode 100644 index 00000000000..5f8261d6a9a --- /dev/null +++ b/ppl/src/test/java/org/opensearch/sql/ppl/AstPlanningTestBase.java @@ -0,0 +1,23 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ppl; + +import org.mockito.Mockito; +import org.opensearch.sql.ast.Node; +import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.ppl.antlr.PPLSyntaxParser; +import org.opensearch.sql.ppl.parser.AstBuilder; + +/** Base class for tests for the AST query planner. */ +public class AstPlanningTestBase { + protected final Settings settings = Mockito.mock(Settings.class); + protected final PPLSyntaxParser parser = new PPLSyntaxParser(); + + protected Node plan(String query) { + AstBuilder astBuilder = new AstBuilder(query, settings); + return astBuilder.visit(parser.parse(query)); + } +} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java index a1823e4befe..33f94dfdea7 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java @@ -64,7 +64,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.mockito.Mockito; import org.opensearch.sql.ast.Node; import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.DataType; @@ -79,20 +78,16 @@ import org.opensearch.sql.ast.tree.RareTopN.CommandType; import org.opensearch.sql.ast.tree.Timechart; import org.opensearch.sql.common.antlr.SyntaxCheckException; -import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.common.setting.Settings.Key; +import org.opensearch.sql.ppl.AstPlanningTestBase; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.ppl.antlr.PPLSyntaxParser; import org.opensearch.sql.utils.SystemIndexUtils; -public class AstBuilderTest { +public class AstBuilderTest extends AstPlanningTestBase { @Rule public ExpectedException exceptionRule = ExpectedException.none(); - private final Settings settings = Mockito.mock(Settings.class); - - private final PPLSyntaxParser parser = new PPLSyntaxParser(); - @Test public void testDynamicSourceClauseThrowsUnsupportedException() { String query = "source=[myindex, logs, fieldIndex=\"test\"]"; @@ -1352,11 +1347,6 @@ protected void assertEqual(String query, String expected) { assertEqual(query, expectedPlan); } - private Node plan(String query) { - AstBuilder astBuilder = new AstBuilder(query, settings); - return astBuilder.visit(parser.parse(query)); - } - private String mappingTable(String indexName) { return SystemIndexUtils.mappingTable(indexName, PPL_SPEC); } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstEquivalenceTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstEquivalenceTest.java new file mode 100644 index 00000000000..d0eb6dec25c --- /dev/null +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstEquivalenceTest.java @@ -0,0 +1,29 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ppl.parser; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.opensearch.sql.ppl.AstPlanningTestBase; + +public class AstEquivalenceTest extends AstPlanningTestBase { + @Test + public void testSpathArgumentDeshuffle() { + assertEquals(plan("source = t | spath path=a input=a"), plan("source = t | spath input=a a")); + } + + @Test + public void testHeadLimitEquivalent() { + assertEquals(plan("source = t | head limit=50"), plan("source = t | head 50")); + } + + @Test + public void testTopLimitEquivalent() { + assertEquals( + plan("source = t | top limit=50 field_name"), plan("source = t | top 50 field_name")); + } +} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/rewrite/SpathRewriteTest.java similarity index 68% rename from ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java rename to ppl/src/test/java/org/opensearch/sql/ppl/rewrite/SpathRewriteTest.java index e97fb51ea90..8b48ddc1db2 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/rewrite/SpathRewriteTest.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package org.opensearch.sql.ppl.utils; +package org.opensearch.sql.ppl.rewrite; import static org.junit.Assert.assertEquals; import static org.opensearch.sql.ast.dsl.AstDSL.eval; @@ -15,23 +15,11 @@ import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import org.junit.Test; -import org.mockito.Mockito; -import org.opensearch.sql.ast.Node; import org.opensearch.sql.ast.tree.Eval; import org.opensearch.sql.ast.tree.SPath; -import org.opensearch.sql.common.setting.Settings; -import org.opensearch.sql.ppl.antlr.PPLSyntaxParser; -import org.opensearch.sql.ppl.parser.AstBuilder; - -public class SPathRewriteTest { - private final Settings settings = Mockito.mock(Settings.class); - private final PPLSyntaxParser parser = new PPLSyntaxParser(); - - private Node plan(String query) { - AstBuilder astBuilder = new AstBuilder(query, settings); - return astBuilder.visit(parser.parse(query)); - } +import org.opensearch.sql.ppl.AstPlanningTestBase; +public class SpathRewriteTest extends AstPlanningTestBase { // Control test to make sure something fundamental hasn't changed about the json_extract parsing @Test public void testEvalControl() { @@ -59,9 +47,4 @@ public void testSpathMissingInputArgumentHandling() { public void testSpathMissingPathArgumentHandling() { plan("source = t | spath input=a output=a"); } - - @Test - public void testSpathArgumentDeshuffle() { - assertEquals(plan("source = t | spath path=a input=a"), plan("source = t | spath input=a a")); - } }