[query] Allow Graphite variadic functions to omit variadic args (#2882)

site/.htmltest.yml

@@ -18,5 +18,7 @@ IgnoreURLs:
 - "youtu.be"
 - "youtube.com"
 - "cassandra.apache.org"
+- "slideshare.net"
+- "meetup.com"
 - "github.com/m3db/m3/tree/docs-test/site/content/docs"
 - "github.com/m3db/m3/tree/master/site/content/docs"

src/query/graphite/native/builtin_functions.go

@@ -2375,7 +2375,6 @@ func init() {
 	})
 	MustRegisterFunction(asPercent).WithDefaultParams(map[uint8]interface{}{
 		2: []*ts.Series(nil), // total
-		3: nil, // nodes
 	})
 	MustRegisterFunction(averageAbove)
 	MustRegisterFunction(averageSeries)
@@ -2406,9 +2405,7 @@ func init() {
 	MustRegisterFunction(groupByNode).WithDefaultParams(map[uint8]interface{}{
 		3: "average", // fname
 	})
-	MustRegisterFunction(groupByNodes).WithDefaultParams(map[uint8]interface{}{
-		3: nil, // nodes
-	})
+	MustRegisterFunction(groupByNodes)
 	MustRegisterFunction(highest).WithDefaultParams(map[uint8]interface{}{
 		2: 1,         // n,
 		3: "average", // f

src/query/graphite/native/compiler.go

@@ -170,6 +170,11 @@ func (c *compiler) compileFunctionCall(fname string, nextToken *lexer.Token) (*f
 	}
 
 	argTypes := fn.in
+	argTypesRequired := len(fn.in)
+	if fn.variadic {
+		// Variadic can avoid specifying the last arg.
+		argTypesRequired--
+	}
 	var args []funcArg
 
 	// build up arguments for function call
@@ -206,7 +211,7 @@ func (c *compiler) compileFunctionCall(fname string, nextToken *lexer.Token) (*f
 	}
 
 	// all required argument types should be filled with values now
-	if len(args) < len(argTypes) {
+	if len(args) < argTypesRequired {
 		variadicComment := ""
 		if fn.variadic {
 			variadicComment = "at least "

src/query/graphite/native/compiler_test.go

@@ -309,9 +309,6 @@ func TestCompileErrors(t *testing.T) {
 		{"aliasByNode()",
 			"invalid expression 'aliasByNode()': invalid number of arguments for aliasByNode;" +
 				" expected at least 2, received 0"},
-		{"aliasByNode(foo.*.zed)", // check that at least 1 param is provided for variadic args
-			"invalid expression 'aliasByNode(foo.*.zed)': invalid number of arguments for " +
-				"aliasByNode; expected at least 2, received 1"},
 		{"aliasByNode(foo.*.zed, 2, false)",
 			"invalid expression 'aliasByNode(foo.*.zed, 2, false)': invalid function call " +
 				"aliasByNode, arg 2: expected a int, received 'false'"},
@@ -327,9 +324,6 @@ func TestCompileErrors(t *testing.T) {
 		{"summarize(foo.bar.baz.quux)",
 			"invalid expression 'summarize(foo.bar.baz.quux)':" +
 				" invalid number of arguments for summarize; expected 4, received 1"},
-		{"sumSeries()", // check that at least 1 series is provided for variadic timeSeriesList
-			"invalid expression 'sumSeries()': invalid number of arguments for sumSeries;" +
-				" expected at least 1, received 0"},
 		{"sumSeries(foo.bar.baz.quux,)",
 			"invalid expression 'sumSeries(foo.bar.baz.quux,)': invalid function call sumSeries, " +
 				"arg 1: invalid expression 'sumSeries(foo.bar.baz.quux,)': ) not valid"},

src/query/graphite/native/engine_test.go

@@ -125,6 +125,14 @@ func TestExecute(t *testing.T) {
 			{"foo.bar.q.zed", "foo.q", 0},
 			{"foo.bar.x.zed", "foo.x", 2},
 		}},
+		{"groupByNodes(foo.bar.*.zed, \"sum\")", false, []queryTestResult{
+			{"foo.bar.*.zed", "foo.bar.*.zed", 3},
+		}},
+		{"groupByNodes(foo.bar.*.zed, \"sum\", 2)", false, []queryTestResult{
+			{"foo.bar.q.zed", "foo.bar.q.zed", 0},
+			{"foo.bar.g.zed", "foo.bar.g.zed", 1},
+			{"foo.bar.x.zed", "foo.bar.x.zed", 2},
+		}},
 	}
 
 	ctx := common.NewContext(common.ContextOptions{Start: time.Now().Add(-1 * time.Hour), End: time.Now(), Engine: engine})
@@ -140,7 +148,7 @@ func TestExecute(t *testing.T) {
 			buildTestSeriesFn(stepSize, queries...))
 
 		expr, err := engine.Compile(test.query)
-		require.Nil(t, err)
+		require.NoError(t, err)
 
 		results, err := expr.Execute(ctx)
 		require.Nil(t, err, "failed to execute %s", test.query)

src/query/graphite/native/functions.go

@@ -422,7 +422,12 @@ func (f *Function) reflectCall(ctx *common.Context, args []reflect.Value) (refle
 	}
 
 	numTypes := len(f.in)
-	if len(in) < numTypes {
+	numRequiredTypes := numTypes
+	if f.variadic {
+		// Variadic can avoid specifying the last arg.
+		numRequiredTypes--
+	}
+	if len(in) < numRequiredTypes {
 		err := fmt.Errorf("call args mismatch: expected at least %d, actual %d",
 			len(f.in), len(in))
 		return reflect.Value{}, err