Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trie: supporting nested value list queries #475

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bom-d-van
Copy link
Member

@bom-d-van bom-d-van commented Jun 29, 2022

Value list (i.e. curl braces) in go-carbon queries is a tricky business. filepath.Glob, trigram, and
trie index all have to work around the issue one way or the other.

Currently, all the three query paths depends on *CarbonserverListener.expandGlobBraces to perform
the expansion. However, it currently does not support nested value lists like {a,b,c,x.{a,b,c}}.

Unlike filepath.Glob and trigram, trie index does not need full expansion, it only needs expansion if
a query contains nested values that are hierarchical (i.e. containing a dir node, another i.e., a dot).
This is also partially due to the current implementation of how trie index handles hierarchical queries.
To be more specific, trieIndex.query does not support queries like x.y.z.{a,nested.subquery}, therefore, a query like that would be expanded by *CarbonserverListener.expandGlobBracesasx.y.z.aandx.y.z.nested.subquery`.

However, the current implementation does not support nested value lists like x.y.z.{a,nested.subquery.{a,b,c}}.

This patch introduces a more through and proper (TM) value list query parser and rewriter for supporting
all expansion cases (hopefully). Like most of the other improvements, only the trie index is supported for now.
Unlike *CarbonserverListener.expandGlobBraces, *CarbonserverListener.expandGlobBracesForTrieIndex would only expand
value list that contains dir/hierarchical nodes. For example, x.y.z.{a,b,nested.subquery.{a,b,c}} is rewritten as
x.y.z.{a,b} and x.y.z.nested.subquery.{a,b,c}, because trie index can handle non-hierarchical value lists
natively.

We should also try to introduce a new expand function that can do full expansion to replace *CarbonserverListener.expandGlobBraces.

Value list (i.e. curl braces) in go-carbon queries is a tricky business. filepath.Glob, trigram, and
trie index all have to work around the issue one way or the other.

Currently, all the three query paths depends on `*CarbonserverListener.expandGlobBraces` to to perform
the expansion. However, it currently does not support nested value lists like `{a,b,c,x.{a,b,c}}`.

Unlike filepath.Glob and trigram, trie index does not need full expansion, it only needs expansion if
a query contains nested values that are hierarchical (i.e. containing a dir node, another i.e., a dot).
This is also partially due to the current implementation of how trie index handles hierarchical queries.
To be more specific, trieIndex.query does not support queries like x.y.z.{a,nested.subquery}`, therefore,
a query like that would be expanded by `*CarbonserverListener.expandGlobBraces` as `x.y.z.a` and
`x.y.z.nested.subquery`.

However, the current implementation does not support nested value lists like `x.y.z.{a,nested.subquery.{a,b,c}}`.

This patch introduces a more through and proper (TM) value list query parser and rewriter for supporting
all expansion cases (hopefully). Like most of the other improvements, only the trie index is supported for now.
Unlike `*CarbonserverListener.expandGlobBraces`, *CarbonserverListener.expandGlobBracesForTrieIndex would only expand
value list that contains dir/hierarchical nodes. For example, `x.y.z.{a,b,nested.subquery.{a,b,c}}` is rewritten as
`x.y.z.{a,b}` and `x.y.z.nested.subquery.{a,b,c}`, because trie index can handle non-hierarchical value lists
natively.

We should also try to introduce a new expand function that can do full expansion to replace `*CarbonserverListener.expandGlobBraces`.
@bom-d-van bom-d-van force-pushed the carbonserver/value-list-bug-fixes branch from 3d6b09b to aa7fcf5 Compare June 29, 2022 07:20
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