Skip to content

fix(parser): allow whitespace between function name and argument list - #943

Draft
npazosmendez wants to merge 4 commits into
mainfrom
njpm/parser-space-before-paren
Draft

fix(parser): allow whitespace between function name and argument list#943
npazosmendez wants to merge 4 commits into
mainfrom
njpm/parser-space-before-paren

Conversation

@npazosmendez

@npazosmendez npazosmendez commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

function ('foo') does not parse. Whether a name starts a function call was decided by testing the byte immediately after it for (, so a name followed by whitespace parsed as a metric name instead.

graphite-web accepts both spellings. Its call is funcname + leftParen, and pyparsing skips whitespace before each element of an And:

https://github.com/graphite-project/graphite-web/blob/1.1.10/webapp/graphite/render/grammar_unsafe.py#L86-L95

The whitespace is consumed only when ( follows, so the non-call path returns its remainder unchanged.


See graphite web test showing a succesfull parse:

$ docker run -d --name gw -p 8080:80 graphiteapp/graphite-statsd >/dev/null && \
until curl -sf -o /dev/null "http://localhost:8080/render?target=x&format=json"; do sleep 2; done && \
curl -sG http://localhost:8080/render --data-urlencode "target=randomWalk ('a')" -d format=json -d from=-5min; echo; \
docker rm -f gw >/dev/null
[{"target": "a", "tags": {"name": "a"}, "datapoints": [[0, 1785872317], [-0.15573856844799938, 1785872377], [-0.38545492969748807, 1785872437], [-0.12892994452545037, 1785872497], [-0.5987020007736302, 1785872557]]}]

parseExprWithoutPipe decided a name was a function call by testing whether
the next byte was '(', so `exclude ('foo')` parsed as the metric name
`exclude`. Piped, that failed with "pipe to not a function"; at top level
the argument list was silently dropped.

graphite-web's grammar builds a call as `funcname + leftParen` in pyparsing,
which skips intervening whitespace, so it accepts both spellings.

Whitespace is consumed only when a '(' follows, so the non-call path returns
its remainder unchanged.
@npazosmendez
npazosmendez force-pushed the njpm/parser-space-before-paren branch from bba834c to 34f6deb Compare August 4, 2026 19:38
…nction calls

The lookahead applied to every parsed name, which broke callers that
re-parse series names and discard the remainder (aliasQuery, applyByNode):
a name like `metric1 (avg: 3)` became a hard parse error. It also turned
metric paths such as `servers.web01.cpu (percent)` into unknown-function
errors, and let groupByNode callbacks like "sum (x)" slip past the EtFunc
guard, silently dropping every group.

The whitespace is now consumed only when the name matches graphite-web's
funcname grammar (Word(alphas+'_', alphanums+'_')) and the argument list
parses; otherwise the name is returned with the remainder untouched, as
before. groupByNode additionally rejects callbacks whose parse leaves a
remainder.
The whitespace lookahead parsed the argument list speculatively and then
re-parsed it in the call branch. Extract the call construction into
parseCall and return its result directly, so each argument list is parsed
once. No behavior change.
@npazosmendez
npazosmendez force-pushed the njpm/parser-space-before-paren branch from 07ad783 to c9ba25b Compare August 5, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant