Allow alternative "by"-clause position in grammar.

In addition to the existing by-clause syntax:

  sum(<expression>) by (<labels>) [keeping_extra]

...this allows the following new syntax:

  sum by (<labels>) [keeping_extra] (<expression>)

Both orderings may be used in a single expression. It is up to the users
to establish guidelines around their usage.

Change-Id: Iba10c9cc5fb6ac62edfcf246d281473e82467992

rules/parser.y

@@ -188,6 +188,12 @@ rule_expr          : '(' rule_expr ')'
                        $$, err = NewVectorAggregation($1, $3, $5, $6)
                        if err != nil { yylex.Error(err.Error()); return 1 }
                      }
+                   | AGGR_OP grouping_opts extra_labels_opts '(' rule_expr ')'
+                     {
+                       var err error
+                       $$, err = NewVectorAggregation($1, $5, $2, $3)
+                       if err != nil { yylex.Error(err.Error()); return 1 }
+                     }
                    /* Yacc can only attach associativity to terminals, so we
                     * have to list all operators here. */
                    | rule_expr ADDITIVE_OP rule_expr

rules/parser.y.go

@@ -75,7 +75,7 @@ const yyEofCode = 1
 const yyErrCode = 2
 const yyMaxDepth = 200
 
-//line parser.y:244
+//line parser.y:250
 
 //line yacctab:1
 var yyExca = []int{
@@ -87,90 +87,91 @@ var yyExca = []int{
 	-2, 10,
 }
 
-const yyNprod = 49
+const yyNprod = 50
 const yyPrivate = 57344
 
 var yyTokenNames []string
 var yyStates []string
 
-const yyLast = 127
+const yyLast = 137
 
 var yyAct = []int{
 
-	51, 64, 48, 47, 24, 54, 6, 20, 10, 49,
+	56, 72, 50, 53, 30, 24, 6, 20, 49, 59,
-	22, 13, 12, 21, 19, 20, 11, 18, 75, 87,
+	22, 10, 51, 18, 13, 12, 21, 19, 20, 11,
-	18, 86, 34, 35, 36, 78, 23, 8, 18, 29,
+	18, 85, 36, 37, 38, 21, 19, 20, 81, 82,
-	7, 46, 50, 10, 49, 28, 13, 12, 21, 19,
+	8, 18, 52, 7, 48, 66, 21, 19, 20, 87,
-	20, 11, 52, 55, 21, 19, 20, 21, 19, 20,
+	18, 10, 51, 31, 13, 12, 60, 55, 28, 11,
-	71, 72, 8, 18, 44, 7, 66, 73, 60, 18,
+	65, 18, 21, 19, 20, 57, 21, 19, 20, 29,
-	27, 37, 18, 67, 70, 10, 19, 20, 13, 12,
+	8, 74, 23, 7, 62, 40, 39, 18, 73, 77,
-	59, 65, 58, 11, 79, 57, 38, 39, 38, 25,
+	76, 18, 80, 75, 10, 19, 20, 13, 12, 79,
-	18, 42, 85, 91, 8, 61, 88, 7, 81, 53,
+	86, 78, 11, 64, 89, 63, 41, 40, 71, 18,
-	41, 63, 9, 17, 77, 69, 82, 33, 31, 43,
+	46, 25, 93, 8, 44, 27, 7, 83, 69, 96,
-	16, 13, 92, 90, 80, 56, 89, 84, 30, 65,
+	94, 91, 58, 43, 9, 17, 54, 31, 92, 35,
-	25, 32, 2, 3, 14, 5, 4, 1, 40, 74,
+	33, 45, 16, 13, 97, 95, 90, 61, 73, 88,
-	76, 15, 26, 62, 68, 83, 45,
+	32, 68, 25, 34, 2, 3, 14, 5, 4, 1,
+	42, 84, 15, 26, 70, 67, 47,
 }
 var yyPact = []int{
 
-	108, -1000, -1000, 59, 82, -1000, 23, 59, 104, 35,
+	120, -1000, -1000, 68, 94, -1000, 41, 68, 116, 70,
-	7, 1, -1000, -1000, -1000, 92, 105, -1000, 89, 59,
+	20, 31, -1000, -1000, -1000, 104, 117, -1000, 101, 68,
-	59, 59, 32, 51, -1000, 66, -1000, 73, 2, 59,
+	68, 68, 37, 60, -1000, 79, -1000, 85, 5, 68,
-	17, -1000, 70, -26, -10, -13, 50, -1000, 104, -1000,
+	93, 19, 30, -1000, 83, -22, -10, -17, 59, -1000,
-	98, -1000, -1000, -1000, 49, 43, -1000, -1000, 23, -1000,
+	116, -1000, 110, -1000, -1000, -1000, 38, 56, -1000, -1000,
-	29, 61, 65, 59, -1000, -1000, -1000, -1000, -1000, 27,
+	41, -1000, 21, 7, -1000, 115, 74, 62, 68, -1000,
-	83, 59, 24, -1000, -1000, 33, -2, -1000, 81, -3,
+	-1000, -1000, -1000, -1000, 35, 95, 68, 52, -1000, 68,
-	23, -1000, 103, 97, 67, 88, -1000, -1000, 101, -1000,
+	2, -1000, -1000, 73, 1, -1000, 93, 10, -1000, 113,
-	-1000, 17, -1000, -8, -1000, 64, -1000, 100, 96, -1000,
+	41, -1000, 112, 109, 80, 100, -1000, -1000, -1000, -1000,
-	60, 95, -1000,
+	-1000, 30, -1000, 78, 108, 76, 107, -1000,
 }
 var yyPgo = []int{
 
-	0, 126, 125, 124, 1, 123, 0, 4, 26, 122,
+	0, 136, 135, 4, 1, 134, 0, 5, 62, 133,
-	2, 3, 121, 120, 119, 92, 118, 117, 116, 115,
+	2, 8, 132, 3, 131, 104, 130, 129, 128, 127,
-	114,
+	126,
 }
 var yyR1 = []int{
 
 	0, 17, 17, 18, 18, 19, 20, 20, 14, 14,
 	12, 12, 15, 15, 6, 6, 6, 5, 5, 4,
 	9, 9, 9, 8, 8, 7, 16, 16, 10, 10,
-	10, 10, 10, 10, 10, 10, 10, 10, 10, 13,
+	10, 10, 10, 10, 10, 10, 10, 10, 10, 10,
-	13, 3, 3, 2, 2, 1, 1, 11, 11,
+	13, 13, 3, 3, 2, 2, 1, 1, 11, 11,
 }
 var yyR2 = []int{
 
 	0, 2, 2, 0, 2, 1, 5, 11, 0, 2,
 	0, 1, 1, 1, 0, 3, 2, 1, 3, 3,
 	0, 2, 3, 1, 3, 3, 1, 1, 3, 3,
-	2, 4, 3, 4, 6, 3, 3, 3, 1, 0,
+	2, 4, 3, 4, 6, 6, 3, 3, 3, 1,
-	1, 0, 4, 1, 3, 1, 3, 1, 1,
+	0, 1, 0, 4, 1, 3, 1, 3, 1, 1,
 }
 var yyChk = []int{
 
 	-1000, -17, 4, 5, -18, -19, -10, 28, 25, -15,
 	6, 14, 10, 9, -20, -12, 18, 11, 30, 16,
 	17, 15, -10, -8, -7, 6, -9, 25, 28, 28,
-	-15, 6, 6, 8, -10, -10, -10, 29, 27, 26,
+	-3, 12, -15, 6, 6, 8, -10, -10, -10, 29,
-	-16, 24, 15, 26, -8, -1, 29, -11, -10, 7,
+	27, 26, -16, 24, 15, 26, -8, -1, 29, -11,
-	-10, -6, 25, 19, 31, -7, 7, 26, 29, 27,
+	-10, 7, -10, -13, 13, 28, -6, 25, 19, 31,
-	29, 24, -5, 26, -4, 6, -10, -11, -3, 12,
+	-7, 7, 26, 29, 27, 29, 28, -2, 6, 24,
-	-10, 26, 27, 24, -14, 20, -13, 13, 28, -4,
+	-5, 26, -4, 6, -10, -11, -3, -10, 29, 27,
-	7, 21, 8, -2, 6, -6, 29, 27, 22, 6,
+	-10, 26, 27, 24, -14, 20, -13, 29, 6, -4,
-	7, 23, 7,
+	7, 21, 8, -6, 22, 7, 23, 7,
 }
 var yyDef = []int{
 
 	0, -2, 3, 0, -2, 2, 5, 0, 0, 20,
-	13, 0, 38, 12, 4, 0, 0, 11, 0, 0,
+	13, 42, 39, 12, 4, 0, 0, 11, 0, 0,
 	0, 0, 0, 0, 23, 0, 30, 0, 0, 0,
-	14, 13, 0, 0, 35, 36, 37, 28, 0, 29,
+	40, 0, 14, 13, 0, 0, 36, 37, 38, 28,
-	0, 26, 27, 21, 0, 0, 32, 45, 47, 48,
+	0, 29, 0, 26, 27, 21, 0, 0, 32, 46,
-	0, 0, 0, 0, 33, 24, 25, 22, 31, 0,
+	48, 49, 0, 0, 41, 0, 0, 0, 0, 33,
-	41, 0, 0, 16, 17, 0, 8, 46, 39, 0,
+	24, 25, 22, 31, 0, 42, 0, 0, 44, 0,
-	6, 15, 0, 0, 0, 0, 34, 40, 0, 18,
+	0, 16, 17, 0, 8, 47, 40, 0, 43, 0,
-	19, 14, 9, 0, 43, 0, 42, 0, 0, 44,
+	6, 15, 0, 0, 0, 0, 34, 35, 45, 18,
-	0, 0, 7,
+	19, 14, 9, 0, 0, 0, 0, 7,
 }
 var yyTok1 = []int{
 
@@ -618,10 +619,10 @@ yydefault:
 			}
 		}
 	case 35:
-		//line parser.y:194
+		//line parser.y:192
 		{
 			var err error
-			yyVAL.ruleNode, err = NewArithExpr(yyS[yypt-1].str, yyS[yypt-2].ruleNode, yyS[yypt-0].ruleNode)
+			yyVAL.ruleNode, err = NewVectorAggregation(yyS[yypt-5].str, yyS[yypt-1].ruleNode, yyS[yypt-4].labelNameSlice, yyS[yypt-3].boolean)
 			if err != nil {
 				yylex.Error(err.Error())
 				return 1
@@ -650,55 +651,65 @@ yydefault:
 	case 38:
 		//line parser.y:212
 		{
-			yyVAL.ruleNode = ast.NewScalarLiteral(yyS[yypt-0].num)
+			var err error
+			yyVAL.ruleNode, err = NewArithExpr(yyS[yypt-1].str, yyS[yypt-2].ruleNode, yyS[yypt-0].ruleNode)
+			if err != nil {
+				yylex.Error(err.Error())
+				return 1
+			}
 		}
 	case 39:
-		//line parser.y:216
+		//line parser.y:218
+		{
+			yyVAL.ruleNode = ast.NewScalarLiteral(yyS[yypt-0].num)
+		}
+	case 40:
+		//line parser.y:222
 		{
 			yyVAL.boolean = false
 		}
-	case 40:
+	case 41:
-		//line parser.y:218
+		//line parser.y:224
 		{
 			yyVAL.boolean = true
 		}
-	case 41:
+	case 42:
-		//line parser.y:222
+		//line parser.y:228
 		{
 			yyVAL.labelNameSlice = clientmodel.LabelNames{}
 		}
-	case 42:
+	case 43:
-		//line parser.y:224
+		//line parser.y:230
 		{
 			yyVAL.labelNameSlice = yyS[yypt-1].labelNameSlice
 		}
-	case 43:
+	case 44:
-		//line parser.y:228
+		//line parser.y:234
 		{
 			yyVAL.labelNameSlice = clientmodel.LabelNames{clientmodel.LabelName(yyS[yypt-0].str)}
 		}
-	case 44:
+	case 45:
-		//line parser.y:230
+		//line parser.y:236
 		{
 			yyVAL.labelNameSlice = append(yyVAL.labelNameSlice, clientmodel.LabelName(yyS[yypt-0].str))
 		}
-	case 45:
+	case 46:
-		//line parser.y:234
+		//line parser.y:240
 		{
 			yyVAL.ruleNodeSlice = []ast.Node{yyS[yypt-0].ruleNode}
 		}
-	case 46:
+	case 47:
-		//line parser.y:236
+		//line parser.y:242
 		{
 			yyVAL.ruleNodeSlice = append(yyVAL.ruleNodeSlice, yyS[yypt-0].ruleNode)
 		}
-	case 47:
+	case 48:
-		//line parser.y:240
+		//line parser.y:246
 		{
 			yyVAL.ruleNode = yyS[yypt-0].ruleNode
 		}
-	case 48:
+	case 49:
-		//line parser.y:242
+		//line parser.y:248
 		{
 			yyVAL.ruleNode = ast.NewStringLiteral(yyS[yypt-0].str)
 		}

rules/rules_test.go

@@ -565,6 +565,37 @@ func TestExpressions(t *testing.T) {
 			fullRanges:     0,
 			intervalRanges: 4,
 		},
+		{
+			// Test alternative "by"-clause order.
+			expr: `sum by (group) (http_requests{job="api-server"})`,
+			output: []string{
+				`http_requests{group="canary"} => 700 @[%v]`,
+				`http_requests{group="production"} => 300 @[%v]`,
+			},
+			fullRanges:     0,
+			intervalRanges: 4,
+		},
+		{
+			// Test alternative "by"-clause order with "keeping_extra".
+			expr: `sum by (group) keeping_extra (http_requests{job="api-server"})`,
+			output: []string{
+				`http_requests{group="canary", job="api-server"} => 700 @[%v]`,
+				`http_requests{group="production", job="api-server"} => 300 @[%v]`,
+			},
+			fullRanges:     0,
+			intervalRanges: 4,
+		},
+		{
+			// Test both alternative "by"-clause orders in one expression.
+			// Public health warning: stick to one form within an expression (or even
+			// in an organization), or risk serious user confusion.
+			expr: `sum(sum by (group) keeping_extra (http_requests{job="api-server"})) by (job)`,
+			output: []string{
+				`http_requests{job="api-server"} => 1000 @[%v]`,
+			},
+			fullRanges:     0,
+			intervalRanges: 4,
+		},
 	}
 
 	storage, closer := newTestStorage(t)