Skip to content

Commit aa7fcf5

Browse files
committed
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`.
1 parent 1182c9c commit aa7fcf5

File tree

4 files changed

+1827
-3
lines changed

4 files changed

+1827
-3
lines changed

carbonserver/carbonserver.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,6 +1407,12 @@ func (listener *CarbonserverListener) expandGlobs(ctx context.Context, query str
14071407
}
14081408

14091409
// TODO(dgryski): add tests
1410+
//
1411+
// TODO(xhu): doesn't support nested braces like {a,b,c,x.{a,b,c}}, should
1412+
// migrate to use the parser and rewriter in trie_value_list.go. however,
1413+
// unlike trie index which supports partial/non-nested value list,
1414+
// filepath.Glob and trigram index doesn't support brace queries at all, so full
1415+
// expansion are needed.
14101416
func (listener *CarbonserverListener) expandGlobBraces(globs []string) ([]string, error) {
14111417
for {
14121418
bracematch := false

carbonserver/trie.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,7 +1490,7 @@ func (ti *trieIndex) countNodes() (count, files, dirs, onec, onefc, onedc int, c
14901490
}
14911491

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

14961496
var slashInBraces, inAlter bool
@@ -1500,17 +1500,24 @@ func (listener *CarbonserverListener) expandGlobsTrie(query string) ([]string, [
15001500
inAlter = true
15011501
case c == '}':
15021502
inAlter = false
1503-
case inAlter && c == '/':
1503+
case inAlter && c == '.':
15041504
slashInBraces = true
15051505
}
15061506
}
15071507
// for complex queries like {a.b.c,x}.o.p.q, fall back to simple expansion
15081508
if slashInBraces {
15091509
var err error
1510-
globs, err = listener.expandGlobBraces(globs)
1510+
globs, err = listener.expandGlobBracesForTrieIndex(query)
15111511
if err != nil {
15121512
return nil, nil, err
15131513
}
1514+
1515+
// TODO: slow? maybe we should make listener.expandGlobBracesForTrieIndex handles / natively?
1516+
for i := 0; i < len(globs); i++ {
1517+
globs[i] = strings.ReplaceAll(globs[i], ".", "/")
1518+
}
1519+
} else {
1520+
globs[0] = strings.ReplaceAll(globs[0], ".", "/")
15141521
}
15151522

15161523
var fidx = listener.CurrentFileIndex()

0 commit comments

Comments
 (0)