feat: Add WITH KEY to show tag keys #20793
Conversation
| indexSet := IndexSet{Indexes: []Index{index}, SeriesFile: s.sfile} | ||
| return indexSet.MeasurementTagKeyValuesByExpr(auth, name, key, expr, keysSorted) | ||
| } | ||
|
|
lesam
changed the title
| // position of the tag key in the keys argument. | ||
| // | ||
| // N.B tagValuesByKeyAndExpr relies on keys being sorted in ascending | ||
| // lexicographic order. | ||
| func (is IndexSet) TagValuesByKeyAndExpr(auth query.Authorizer, name []byte, keys []string, expr influxql.Expr, fieldset *MeasurementFieldSet) ([]map[string]struct{}, error) { |
| @@ -1663,7 +1656,7 @@ func (s *Store) TagKeys(auth query.Authorizer, shardIDs []uint64, cond influxql. | |||
| for _, name := range names { | |||
|
|
|||
| // Build keyset over all indexes for measurement. | |||
| tagKeySet, err := is.MeasurementTagKeysByExpr(name, nil) | |||
| tagKeySet, err := is.MeasurementTagKeysByExpr(name, tagKeyExpr) | |||
There was a problem hiding this comment.
Same behaviour as 'show tag value with key'
| @@ -1845,7 +1859,7 @@ func (s *Store) TagValues(auth query.Authorizer, shardIDs []uint64, cond influxq | |||
| // filter. | |||
| for _, name := range names { | |||
| // Determine a list of keys from condition. | |||
| keySet, err := is.MeasurementTagKeysByExpr(name, cond) | |||
| keySet, err := is.MeasurementTagKeysByExpr(name, tagKeyExpr) | |||
There was a problem hiding this comment.
This worked before because we always had a _tagKey field (enforced by the parser). But if you don't have any binary expressions with _tagKey but you do have a non-nil expression this returns an empty set.
|
|
||
| nextResult := mergeTagValues(idxBuf, allResults[i:j+1]...) | ||
| i = j + 1 | ||
| for _, r := range allResults { |
There was a problem hiding this comment.
Probably this used to be needed before iterators over IndexSet? Now the merge is done within the iterator.
| // seriesByExprIterator doesn't handle | ||
| lit, ok := e.Expr.(*influxql.BooleanLiteral) | ||
| if !ok { | ||
| return nil, fmt.Errorf("Expression too complex for metaquery: %v", e.Expr) |
There was a problem hiding this comment.
Note this new error condition.
tsdb/exprs.go
Outdated
| passExpr = ExprsToConjunction(passSlice...) | ||
| failExpr = ExprsToConjunction(failSlice...) | ||
| return passExpr, failExpr, nil | ||
| } |
There was a problem hiding this comment.
The function looks fine but we use the term "partition" in a lot of places in the code and this partitioning seems unrelated to any of those. I'm not sure what better word would work but it's something to consider.
There was a problem hiding this comment.
Maybe. We use the same term in the flux code though (PartitionPredicate)
Also remove some dead code
| idxBuf := make([][2]int, 0, len(is.Indexes)) | ||
| for i < len(allResults) { | ||
| result := make([]TagValues, 0, maxMeasurements) | ||
| for _, r := range allResults { | ||
| // check for timeouts | ||
| select { |
There was a problem hiding this comment.
Fixed this merge conflict with @davidby-influx 's PR merged yesterday.
lesam
force-pushed
the
fix-tag-keys
branch
2 times, most recently
from
c229e94 to
e53e9db
Compare
| // seriesByExprIterator doesn't handle | ||
| lit, ok := e.Expr.(*influxql.BooleanLiteral) | ||
| if !ok { | ||
| return nil, fmt.Errorf("Expression too complex for metaquery: %v", e.Expr) |
There was a problem hiding this comment.
Wondering if there is a more informative error message we could give the user here. "Meta queries don't support..."?
There was a problem hiding this comment.
I'm not sure what the exact rules are unfortunately, and I'm worried this would be hard to maintain if we add more support in the various places we partially evaluate the query.
| }, | ||
| &Query{ | ||
| name: "show tag keys from regex with key in", | ||
| command: "SHOW TAG KEYS FROM /[cg]pu/ WITH KEY IN (host, region) ", |
* fix: Change from RewriteExpr to PartitionExpr Also remove some dead code * feat: WITH KEY implementation * feat: query rewriting for WITH KEY in SHOW TAG KEYS
Closes #15993