Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
trie: supporting nested value list queries
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
commit aa7fcf5622802119d9b0dd2cd31c3a096e5b4c06
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