PromQL: Support trailing commas in grouping opts (#6480)

* PromQL: Support trailing commas in grouping opts

Fixes #6470

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

docs/querying/basics.md

@@ -74,8 +74,8 @@ name:
 
     http_requests_total
 
-It is possible to filter these time series further by appending a set of labels
+It is possible to filter these time series further by appending a comma separated list of label
-to match in curly braces (`{}`).
+matchers in curly braces (`{}`).
 
 This example selects only those time series with the `http_requests_total`
 metric name that also have the `job` label set to `prometheus` and their

docs/querying/operators.md

@@ -205,12 +205,17 @@ or
 
     <aggr-op>([parameter,] <vector expression>) [without|by (<label list>)]
 
-`parameter` is only required for `count_values`, `quantile`, `topk` and
+`label list` is a list of unquoted labels that may include a trailing comma, i.e.
-`bottomk`. `without` removes the listed labels from the result vector, while
+both `(label1, label2)` and `(label1, label2,)` are valid syntax.
+
+`without` removes the listed labels from the result vector, while
 all other labels are preserved the output. `by` does the opposite and drops
 labels that are not listed in the `by` clause, even if their label values are
 identical between all elements of the vector.
 
+`parameter` is only required for `count_values`, `quantile`, `topk` and
+`bottomk`.
+
 `count_values` outputs one time series per unique sample value. Each series has
 an additional label. The name of that label is given by the aggregation
 parameter, and the label value is the unique sample value.  The value of each

promql/generated_parser.y

@@ -225,6 +225,8 @@ label_set_item  :
 grouping_labels :
                 LEFT_PAREN grouping_label_list RIGHT_PAREN
                         { $$ = $2 }
+                | LEFT_PAREN grouping_label_list COMMA RIGHT_PAREN
+                        { $$ = $2 }
                 | LEFT_PAREN RIGHT_PAREN
                         { $$ = []string{} }
                 | error

promql/generated_parser.y.go

@@ -175,7 +175,7 @@ const yyEofCode = 1
 const yyErrCode = 2
 const yyInitialStackSize = 16
 
-//line promql/generated_parser.y:284
+//line promql/generated_parser.y:286
 
 //line yacctab:1
 var yyExca = [...]int{
@@ -189,59 +189,62 @@ var yyExca = [...]int{
 
 const yyPrivate = 57344
 
-const yyLast = 117
+const yyLast = 142
 
 var yyAct = [...]int{
 
-	35, 6, 33, 27, 72, 36, 37, 83, 32, 79,
+	35, 27, 33, 63, 22, 36, 37, 6, 84, 83,
-	22, 29, 75, 24, 10, 8, 28, 29, 23, 1,
+	79, 75, 1, 29, 10, 70, 31, 8, 28, 73,
-	73, 80, 28, 76, 82, 74, 78, 26, 24, 38,
+	17, 11, 72, 80, 74, 68, 82, 78, 69, 38,
-	39, 40, 19, 23, 31, 35, 71, 17, 21, 18,
+	39, 40, 19, 25, 34, 64, 65, 12, 62, 18,
-	36, 37, 11, 41, 42, 43, 44, 45, 46, 47,
+	20, 66, 67, 41, 42, 43, 44, 45, 46, 47,
-	48, 49, 50, 51, 25, 34, 52, 53, 12, 54,
+	48, 49, 50, 51, 71, 7, 52, 53, 0, 54,
-	55, 56, 57, 58, 38, 39, 40, 2, 3, 4,
+	55, 56, 57, 58, 35, 77, 0, 0, 61, 36,
-	5, 77, 63, 81, 62, 20, 7, 84, 41, 42,
+	37, 81, 32, 2, 3, 4, 5, 85, 59, 24,
-	43, 44, 45, 46, 47, 48, 49, 50, 51, 70,
+	29, 60, 24, 0, 23, 28, 0, 23, 0, 76,
-	61, 52, 53, 0, 54, 55, 56, 57, 58, 68,
+	26, 14, 21, 38, 39, 40, 16, 15, 0, 0,
-	59, 9, 69, 60, 64, 65, 13, 14, 0, 0,
+	10, 0, 0, 0, 0, 0, 0, 41, 42, 43,
-	66, 67, 16, 15, 30, 0, 10,
+	44, 45, 46, 47, 48, 49, 50, 51, 0, 0,
+	52, 53, 0, 54, 55, 56, 57, 58, 9, 0,
+	0, 0, 0, 13, 0, 0, 0, 0, 0, 0,
+	0, 30,
 }
 var yyPact = [...]int{
 
-	-1, -1000, 4, 3, 105, 30, -1000, -1000, 26, -1000,
+	5, -1000, 6, 3, 89, 30, -1000, -1000, 80, -1000,
-	15, -1000, 3, -1000, -1000, -1000, -1000, -1000, -2, -1000,
+	78, -1000, 3, -1000, -1000, -1000, -1000, -1000, 62, -1000,
-	88, -1000, -1000, 70, -1000, 87, -1000, -1000, 2, -1000,
+	66, -1000, -1000, 1, -1000, 13, -1000, -1000, 20, -1000,
-	-1000, 10, -1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000,
+	-1000, 9, -1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000,
 	-1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000,
 	-1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000, -1000,
-	11, -1000, 7, -1000, -1000, -1000, -1000, -1000, -1000, 9,
+	77, -1000, 8, -1000, -1000, -1000, -1000, -1000, -1000, 11,
-	-1000, 5, -1000, -1000, 33, -1000, -1000, -1000, -1000, -1000,
+	-1000, 7, -1000, -1000, -2, -1000, -1000, -1000, -1000, -1000,
-	-1000, -1000, -1000, -1000, -1000,
+	-1000, -1000, -1000, -1000, -1000, -1000,
 }
 var yyPgo = [...]int{
 
-	0, 76, 75, 10, 74, 58, 2, 55, 54, 101,
+	0, 55, 40, 4, 38, 37, 2, 34, 33, 128,
-	42, 3, 37, 34, 19,
+	21, 1, 20, 16, 12,
 }
 var yyR1 = [...]int{
 
 	0, 14, 14, 14, 14, 14, 1, 1, 1, 2,
 	2, 2, 3, 3, 3, 3, 4, 4, 4, 4,
 	10, 10, 10, 5, 5, 9, 9, 9, 9, 8,
-	8, 8, 11, 11, 11, 11, 12, 12, 12, 13,
+	8, 8, 11, 11, 11, 11, 12, 12, 12, 12,
-	13, 13, 6, 6, 7, 7, 7, 7, 7, 7,
+	13, 13, 13, 6, 6, 7, 7, 7, 7, 7,
 	7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
-	7, 7, 7, 7, 7, 7, 7,
+	7, 7, 7, 7, 7, 7, 7, 7,
 }
 var yyR2 = [...]int{
 
 	0, 2, 2, 2, 2, 1, 3, 4, 2, 3,
 	1, 2, 3, 3, 2, 1, 1, 1, 1, 1,
 	2, 1, 1, 1, 1, 3, 4, 2, 0, 3,
-	1, 2, 3, 3, 2, 1, 3, 2, 1, 3,
+	1, 2, 3, 3, 2, 1, 3, 4, 2, 1,
-	1, 2, 1, 1, 1, 1, 1, 1, 1, 1,
+	3, 1, 2, 1, 1, 1, 1, 1, 1, 1,
 	1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-	1, 1, 1, 1, 1, 1, 1,
+	1, 1, 1, 1, 1, 1, 1, 1,
 }
 var yyChk = [...]int{
 
@@ -253,19 +256,19 @@ var yyChk = [...]int{
 	54, 55, 58, 59, 61, 62, 63, 64, 65, 12,
 	15, 2, -4, 2, 34, 35, 40, 41, 12, 15,
 	2, 34, 2, 10, 15, 2, 12, -3, 19, 2,
-	12, -11, 19, 2, -6,
+	12, -11, 19, 2, 10, -6,
 }
 var yyDef = [...]int{
 
 	0, -2, 0, 28, -2, 0, 5, 1, 0, 2,
-	0, 3, 28, 21, 22, 23, 24, 4, 0, 38,
+	0, 3, 28, 21, 22, 23, 24, 4, 0, 39,
 	0, 8, 10, 0, 15, 0, 27, 30, 0, 35,
-	20, 0, 37, 40, 42, 43, 44, 45, 46, 47,
+	20, 0, 38, 41, 43, 44, 45, 46, 47, 48,
-	48, 49, 50, 51, 52, 53, 54, 55, 56, 57,
+	49, 50, 51, 52, 53, 54, 55, 56, 57, 58,
-	58, 59, 60, 61, 62, 63, 64, 65, 66, 6,
+	59, 60, 61, 62, 63, 64, 65, 66, 67, 6,
 	0, 11, 0, 14, 16, 17, 18, 19, 25, 0,
-	31, 0, 34, 36, 0, 41, 7, 9, 12, 13,
+	31, 0, 34, 36, 0, 42, 7, 9, 12, 13,
-	26, 29, 32, 33, 39,
+	26, 29, 32, 33, 37, 40,
 }
 var yyTok1 = [...]int{
 
@@ -841,47 +844,53 @@ yydefault:
 			yyVAL.strings = yyDollar[2].strings
 		}
 	case 37:
-		yyDollar = yyS[yypt-2 : yypt+1]
+		yyDollar = yyS[yypt-4 : yypt+1]
 //line promql/generated_parser.y:229
+		{
+			yyVAL.strings = yyDollar[2].strings
+		}
+	case 38:
+		yyDollar = yyS[yypt-2 : yypt+1]
+//line promql/generated_parser.y:231
 		{
 			yyVAL.strings = []string{}
 		}
-	case 38:
+	case 39:
 		yyDollar = yyS[yypt-1 : yypt+1]
-//line promql/generated_parser.y:231
+//line promql/generated_parser.y:233
 		{
 			yylex.(*parser).unexpected("grouping opts", "\"(\"")
 		}
-	case 39:
+	case 40:
 		yyDollar = yyS[yypt-3 : yypt+1]
-//line promql/generated_parser.y:237
+//line promql/generated_parser.y:239
 		{
 			yyVAL.strings = append(yyDollar[1].strings, yyDollar[3].item.Val)
 		}
-	case 40:
+	case 41:
 		yyDollar = yyS[yypt-1 : yypt+1]
-//line promql/generated_parser.y:239
+//line promql/generated_parser.y:241
 		{
 			yyVAL.strings = []string{yyDollar[1].item.Val}
 		}
-	case 41:
+	case 42:
 		yyDollar = yyS[yypt-2 : yypt+1]
-//line promql/generated_parser.y:241
+//line promql/generated_parser.y:243
 		{
 			yylex.(*parser).unexpected("grouping opts", "\",\" or \"}\"")
 		}
-	case 42:
+	case 43:
 		yyDollar = yyS[yypt-1 : yypt+1]
-//line promql/generated_parser.y:246
+//line promql/generated_parser.y:248
 		{
 			if !isLabel(yyDollar[1].item.Val) {
 				yylex.(*parser).unexpected("grouping opts", "label")
 			}
 			yyVAL.item = yyDollar[1].item
 		}
-	case 43:
+	case 44:
 		yyDollar = yyS[yypt-1 : yypt+1]
-//line promql/generated_parser.y:253
+//line promql/generated_parser.y:255
 		{
 			yylex.(*parser).unexpected("grouping opts", "label")
 		}

promql/parse_test.go

@@ -1190,6 +1190,30 @@ var testExpr = []struct {
 			},
 			Grouping: []string{},
 		},
+	}, {
+		input: "sum by (foo,bar,)(some_metric)",
+		expected: &AggregateExpr{
+			Op: SUM,
+			Expr: &VectorSelector{
+				Name: "some_metric",
+				LabelMatchers: []*labels.Matcher{
+					mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"),
+				},
+			},
+			Grouping: []string{"foo", "bar"},
+		},
+	}, {
+		input: "sum by (foo,)(some_metric)",
+		expected: &AggregateExpr{
+			Op: SUM,
+			Expr: &VectorSelector{
+				Name: "some_metric",
+				LabelMatchers: []*labels.Matcher{
+					mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"),
+				},
+			},
+			Grouping: []string{"foo"},
+		},
 	}, {
 		input: "topk(5, some_metric)",
 		expected: &AggregateExpr{
@@ -1232,6 +1256,14 @@ var testExpr = []struct {
 		input:  "sum without(==)(some_metric)",
 		fail:   true,
 		errMsg: "unexpected <op:==> in grouping opts, expected label",
+	}, {
+		input:  "sum without(,)(some_metric)",
+		fail:   true,
+		errMsg: `unexpected "," in grouping opts, expected label`,
+	}, {
+		input:  "sum without(foo,,)(some_metric)",
+		fail:   true,
+		errMsg: `unexpected "," in grouping opts, expected label`,
 	}, {
 		input:  `sum some_metric by (test)`,
 		fail:   true,