From b38506a02486e4676f0ee7913d0797801763e92d Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Thu, 12 Nov 2020 01:43:46 -0500 Subject: [PATCH 1/4] [query] Allow Graphite variadic functions to omit variadic args --- src/query/graphite/native/builtin_functions.go | 5 +---- src/query/graphite/native/compiler.go | 8 +++++++- src/query/graphite/native/engine_test.go | 10 +++++++++- src/query/graphite/native/functions.go | 7 ++++++- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index f757ddb528..bc65fc74d6 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/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 diff --git a/src/query/graphite/native/compiler.go b/src/query/graphite/native/compiler.go index bbca546914..20ecc87838 100644 --- a/src/query/graphite/native/compiler.go +++ b/src/query/graphite/native/compiler.go @@ -170,6 +170,12 @@ func (c *compiler) compileFunctionCall(fname string, nextToken *lexer.Token) (*f } argTypes := fn.in + argTypesN := len(fn.in) + argTypesRequiredN := argTypesN + if fn.variadic { + // Variadic can avoid specifying the last arg. + argTypesRequiredN-- + } var args []funcArg // build up arguments for function call @@ -206,7 +212,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) < argTypesRequiredN { variadicComment := "" if fn.variadic { variadicComment = "at least " diff --git a/src/query/graphite/native/engine_test.go b/src/query/graphite/native/engine_test.go index cc4f9d229a..55d2c8b5c2 100644 --- a/src/query/graphite/native/engine_test.go +++ b/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) diff --git a/src/query/graphite/native/functions.go b/src/query/graphite/native/functions.go index b68fa5bf10..b0109922ac 100644 --- a/src/query/graphite/native/functions.go +++ b/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 From 67fed59d0c9d12551aba3ccccffd23312b8185fb Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Thu, 12 Nov 2020 01:46:28 -0500 Subject: [PATCH 2/4] Rename --- src/query/graphite/native/compiler.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/query/graphite/native/compiler.go b/src/query/graphite/native/compiler.go index 20ecc87838..fec31a438b 100644 --- a/src/query/graphite/native/compiler.go +++ b/src/query/graphite/native/compiler.go @@ -170,11 +170,10 @@ func (c *compiler) compileFunctionCall(fname string, nextToken *lexer.Token) (*f } argTypes := fn.in - argTypesN := len(fn.in) - argTypesRequiredN := argTypesN + argTypesRequired := len(fn.in) if fn.variadic { // Variadic can avoid specifying the last arg. - argTypesRequiredN-- + argTypesRequired-- } var args []funcArg @@ -212,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) < argTypesRequiredN { + if len(args) < argTypesRequired { variadicComment := "" if fn.variadic { variadicComment = "at least " From 0765cffb84db28587aff49d09f99e0ab1d25da13 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Thu, 19 Nov 2020 23:38:58 -0500 Subject: [PATCH 3/4] Remove the variadic errors --- src/query/graphite/native/compiler_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/query/graphite/native/compiler_test.go b/src/query/graphite/native/compiler_test.go index 40f917082f..589bdf1ee3 100644 --- a/src/query/graphite/native/compiler_test.go +++ b/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"}, From 82263b20ffb2add1ce7658fecf16d1826ab22974 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Fri, 20 Nov 2020 00:10:18 -0500 Subject: [PATCH 4/4] Fix docs --- site/.htmltest.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/.htmltest.yml b/site/.htmltest.yml index c51da7eb55..99d5af9b82 100644 --- a/site/.htmltest.yml +++ b/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" \ No newline at end of file