Span expression should always be first in by list if exist

penghuo · penghuo · commit 5849d9f26a68 · 2022-02-18T09:38:06.000-08:00
Signed-off-by: penghuo &lt;penghuo@gmail.com&gt;
diff --git a/docs/user/ppl/cmd/stats.rst b/docs/user/ppl/cmd/stats.rst
@@ -39,7 +39,7 @@ stats <aggregation>... [by-clause]
 
 * by-clause: optional.
 
- * Syntax: by [field]... [span-expression].
+ * Syntax: by [span-expression,] [field,]...
  * Description: The by clause could be the fields and expressions like scalar functions and aggregation functions. Besides, the span clause can be used to split specific field into buckets in the same interval, the stats then does the aggregation by these span buckets.
  * Default: If no <by-clause> is specified, the stats command returns only one row, which is the aggregation over the entire result set.
 
@@ -372,7 +372,7 @@ The example gets the count of age by the interval of 10 years and group by gende
 
 PPL query::
 
-    os> source=accounts | stats count() as cnt by gender span(age, 5) as age_span
+    os> source=accounts | stats count() as cnt by span(age, 5) as age_span, gender
     fetched rows / total rows = 3/3
     +-------+------------+----------+
     | cnt   | age_span   | gender   |
diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4
@@ -101,7 +101,7 @@ byClause
 statsByClause
     : BY fieldList
     | BY bySpanClause
-    | BY fieldList bySpanClause
+    | BY bySpanClause COMMA fieldList
     ;
 
 bySpanClause
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
@@ -294,7 +294,7 @@ public void testStatsCommandWithSpan() {
             defaultStatsArgs()
         ));
 
-    assertEqual("source=t | stats avg(price) by b span(timestamp, 1h)",
+    assertEqual("source=t | stats avg(price) by span(timestamp, 1h), b",
         agg(
             relation("t"),
             exprList(
@@ -306,7 +306,7 @@ public void testStatsCommandWithSpan() {
             defaultStatsArgs()
         ));
 
-    assertEqual("source=t | stats avg(price) by f1, f2 span(timestamp, 1h)",
+    assertEqual("source=t | stats avg(price) by span(timestamp, 1h), f1, f2",
         agg(
             relation("t"),
             exprList(

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ byClause`
`101`	`101`	`statsByClause`
`102`	`102`	`: BY fieldList`
`103`	`103`	`\| BY bySpanClause`
`104`		`- \| BY fieldList bySpanClause`
	`104`	`+ \| BY bySpanClause COMMA fieldList`
`105`	`105`	`;`
`106`	`106`
`107`	`107`	`bySpanClause`