Skip to content

Commit

Permalink
trie: supporting nested value list queries
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
bom-d-van committed Jun 29, 2022
1 parent 1182c9c commit aa7fcf5
Show file tree
Hide file tree
Showing 4 changed files with 1,827 additions and 3 deletions.
6 changes: 6 additions & 0 deletions carbonserver/carbonserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,12 @@ func (listener *CarbonserverListener) expandGlobs(ctx context.Context, query str
}

// TODO(dgryski): add tests
//
// TODO(xhu): doesn't support nested braces like {a,b,c,x.{a,b,c}}, should
// migrate to use the parser and rewriter in trie_value_list.go. however,
// unlike trie index which supports partial/non-nested value list,
// filepath.Glob and trigram index doesn't support brace queries at all, so full
// expansion are needed.
func (listener *CarbonserverListener) expandGlobBraces(globs []string) ([]string, error) {
for {
bracematch := false
Expand Down
13 changes: 10 additions & 3 deletions carbonserver/trie.go
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,7 @@ func (ti *trieIndex) countNodes() (count, files, dirs, onec, onefc, onedc int, c
}

func (listener *CarbonserverListener) expandGlobsTrie(query string) ([]string, []bool, error) {
query = strings.ReplaceAll(query, ".", "/")
// query = strings.ReplaceAll(query, ".", "/")
globs := []string{query}

var slashInBraces, inAlter bool
Expand All @@ -1500,17 +1500,24 @@ func (listener *CarbonserverListener) expandGlobsTrie(query string) ([]string, [
inAlter = true
case c == '}':
inAlter = false
case inAlter && c == '/':
case inAlter && c == '.':
slashInBraces = true
}
}
// for complex queries like {a.b.c,x}.o.p.q, fall back to simple expansion
if slashInBraces {
var err error
globs, err = listener.expandGlobBraces(globs)
globs, err = listener.expandGlobBracesForTrieIndex(query)
if err != nil {
return nil, nil, err
}

// TODO: slow? maybe we should make listener.expandGlobBracesForTrieIndex handles / natively?
for i := 0; i < len(globs); i++ {
globs[i] = strings.ReplaceAll(globs[i], ".", "/")
}
} else {
globs[0] = strings.ReplaceAll(globs[0], ".", "/")
}

var fidx = listener.CurrentFileIndex()
Expand Down
Loading

0 comments on commit aa7fcf5

Please sign in to comment.