-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented the tag values iterator for SHOW TAG VALUES
#5853
Conversation
I had to add another filtering function which was a bit weird and I would prefer another solution with more code reuse, but I couldn't think of a way that didn't break another part of functionality. |
SHOW TAG VALUES
SHOW TAG VALUES
Not quite done yet. It still needs to handle more functionality that I didn't realize needed to be covered. This kind of call has to be supported: |
779c909
to
0d80497
Compare
SHOW TAG VALUES
SHOW TAG VALUES
I think it's working now. @jwilder @e-dard @benbjohnson please review. I'm not sure how happy I am with the end result, but I do believe it works. This change also affects backwards compatibility so I would like any feedback regarding that. The change to the output is located in the commit message. |
n.Condition = cond.(Expr) | ||
} else { | ||
n.Condition = nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a RewriteExpr()
that accepts and returns an Expr
so that you don't have to do the nil
cast everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a good idea. I can do that.
Overall it lgtm. Can you post a before/after example of the output and verify with @pauldix about the change. I'm ok with it. |
0d80497
to
805b311
Compare
`SHOW TAG VALUES` output has been modified to print the measurement name for every measurement and to return the output in two columns: key and value. An example output might be: > SHOW TAG VALUES WITH KEY IN (host, region) name: cpu --------- key value host server01 region useast name: mem --------- key value host server02 region useast `measurementsByExpr` has been taught how to handle reserved keys (ones with an underscore at the beginning) to allow reusing that function and skipping over expressions that don't matter to the call. Fixes #5593.
805b311
to
2f0e246
Compare
Change in output from:
to
|
new output format looks good to me. Does it support @mark-rushakoff: this change will almost certainly break Chronograf so we'll have to update for it. |
@pauldix yep, it supports that.
|
Implemented the tag values iterator for `SHOW TAG VALUES`
Fixes #5593.