trie: supporting nested value list queries - #475
Draft
bom-d-van wants to merge 1 commit into
Draft
Conversation
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
force-pushed
the
carbonserver/value-list-bug-fixes
branch
from
June 29, 2022 07:20
3d6b09b to
aa7fcf5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.expandGlobBracesto performthe 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 expandvalue list that contains dir/hierarchical nodes. For example,
x.y.z.{a,b,nested.subquery.{a,b,c}}is rewritten asx.y.z.{a,b}andx.y.z.nested.subquery.{a,b,c}, because trie index can handle non-hierarchical value listsnatively.
We should also try to introduce a new expand function that can do full expansion to replace
*CarbonserverListener.expandGlobBraces.