From 8ca7394bc4ab9cc7deb1f60310997d792bd34160 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Tue, 2 Jun 2026 23:51:27 -0700 Subject: [PATCH 1/2] fix: parse digit-dash root metric names instead of failing as floats --- pkg/parser/parser.go | 15 ++++++++--- pkg/parser/parser_test.go | 52 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index ef9135d14..7ac2c8b81 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -613,10 +613,17 @@ func parseExprWithoutPipe(e string) (Expr, string, error) { } if '0' <= e[0] && e[0] <= '9' || e[0] == '-' || e[0] == '+' { - val, valStr, e, err := parseConst(e) - r, _ := utf8.DecodeRuneInString(e) - if !unicode.IsLetter(r) { - return &expr{val: val, etype: EtConst, valStr: valStr}, e, err + val, valStr, rest, err := parseConst(e) + // Only treat the token as a numeric constant when parseConst actually + // succeeded. A failure means the greedily-slurped run of [0-9.+-eE] + // characters is not a valid float (e.g. a root metric name like + // "587-AL"); in that case fall through to parseName so the original + // token is parsed as a metric name, matching graphite-web (issue #524). + if err == nil { + r, _ := utf8.DecodeRuneInString(rest) + if !unicode.IsLetter(r) { + return &expr{val: val, etype: EtConst, valStr: valStr}, rest, nil + } } } diff --git a/pkg/parser/parser_test.go b/pkg/parser/parser_test.go index 1966b31bf..d8f2b0b57 100644 --- a/pkg/parser/parser_test.go +++ b/pkg/parser/parser_test.go @@ -165,6 +165,58 @@ func TestParseExpr(t *testing.T) { }, }, + // Issue #524: a root metric name that starts with one or more digits + // followed by a dash (e.g. "587-AL") must parse as a metric name, the + // same as graphite-web, instead of failing ParseFloat on "587-". + { + "587-AL.random.a", + &expr{target: "587-AL.random.a"}, + }, + { + "587-AL", + &expr{target: "587-AL"}, + }, + // Guard against over-correction: these already parsed as names and + // must keep doing so. + { + "AAA-587", + &expr{target: "AAA-587"}, + }, + { + "587AAA", + &expr{target: "587AAA"}, + }, + { + "AAA587", + &expr{target: "AAA587"}, + }, + // Genuine numeric constants must still parse as EtConst. + { + "-1", + &expr{val: -1, etype: EtConst, valStr: "-1"}, + }, + { + "+2.5", + &expr{val: 2.5, etype: EtConst, valStr: "+2.5"}, + }, + { + "1e3", + &expr{val: 1000, etype: EtConst, valStr: "1e3"}, + }, + // A function call with a digit-dash series arg parses without error and + // preserves the inner metric name. + { + "sumSeries(587-AL.random.*)", + &expr{ + target: "sumSeries", + etype: EtFunc, + args: []*expr{ + {target: "587-AL.random.*"}, + }, + argString: "587-AL.random.*", + }, + }, + { "func1(metric1, -3 , 'foo' )", &expr{ From d726d6ac46ccc1d43d435b77aa291268cdd27c32 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 3 Jun 2026 21:10:45 -0700 Subject: [PATCH 2/2] refactor: extract parseConstExpr helper from parseExprWithoutPipe Pulls the numeric-constant parsing branch into its own function to reduce the cognitive complexity flagged by static analysis, preserving the digit-dash root metric fallback behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/parser/parser.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index 7ac2c8b81..e2dd8015c 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -612,18 +612,10 @@ func parseExprWithoutPipe(e string) (Expr, string, error) { return nil, "", ErrMissingExpr } - if '0' <= e[0] && e[0] <= '9' || e[0] == '-' || e[0] == '+' { - val, valStr, rest, err := parseConst(e) - // Only treat the token as a numeric constant when parseConst actually - // succeeded. A failure means the greedily-slurped run of [0-9.+-eE] - // characters is not a valid float (e.g. a root metric name like - // "587-AL"); in that case fall through to parseName so the original - // token is parsed as a metric name, matching graphite-web (issue #524). - if err == nil { - r, _ := utf8.DecodeRuneInString(rest) - if !unicode.IsLetter(r) { - return &expr{val: val, etype: EtConst, valStr: valStr}, rest, nil - } + if IsDigit(e[0]) || e[0] == '-' || e[0] == '+' { + exp, rest, ok := parseConstExpr(e) + if ok { + return exp, rest, nil } } @@ -662,6 +654,25 @@ func parseExprWithoutPipe(e string) (Expr, string, error) { return &expr{target: name}, e, nil } +func parseConstExpr(e string) (*expr, string, bool) { + val, valStr, rest, err := parseConst(e) + if err != nil { + // A failure means the greedily-slurped run of [0-9.+-eE] characters is + // not a valid float, e.g. a root metric name like "587-AL". Let the + // caller parse the original token as a metric name instead. + return nil, e, false + } + + if rest != "" { + r, size := utf8.DecodeRuneInString(rest) + if size > 0 && unicode.IsLetter(r) { + return nil, e, false + } + } + + return &expr{val: val, etype: EtConst, valStr: valStr}, rest, true +} + func parseExprInner(e string) (Expr, string, error) { exp, e, err := parseExprWithoutPipe(e) if err != nil {