fix: forward-port changes to tag values/key for better predicate handling #22500
Conversation
| @@ -0,0 +1,128 @@ | |||
| package influxdb_test | |||
There was a problem hiding this comment.
These tests are included here to show that the changes do fix #22385, but should be moved to the Flux repo when this merges and later removed when we update to a version of flux that includes these tests.
There was a problem hiding this comment.
This might be a good TODO comment to put in the test itself.
| @@ -108,7 +108,7 @@ require ( | |||
| github.com/influxdata/httprouter v1.3.1-0.20191122104820-ee83e2772f69 | |||
| github.com/influxdata/influx-cli/v2 v2.1.1-0.20210813175002-13799e7662c0 | |||
| github.com/influxdata/influxdb-client-go/v2 v2.3.1-0.20210518120617-5d1fff431040 // indirect | |||
| github.com/influxdata/influxql v0.0.0-20180925231337-1cbfca8e56b6 | |||
| github.com/influxdata/influxql v1.1.1-0.20210223160523-b6ab99450c93 | |||
There was a problem hiding this comment.
Are there any other meaningful changes this pulls in?
There was a problem hiding this comment.
Also do we need this change if we're not pulling in the WITH KEY work?
There was a problem hiding this comment.
Also we should probably have an issue to forward-port WITH KEY so we can keep track of what's done/not done/why.
There was a problem hiding this comment.
Bumping the InfluxQL version was necessary to be able to use the PartitionExpr from the InfluxQL package which was added recently. That's what really does the magic that fixes the tag values metaquery issue.
I created an issue for forwarding porting this and added a comment explaining what this PR partially ports: #22499
As far as InfluxQL changes, there's only a few - a couple of which seem somewhat meaningful:
- feat: support advanced bound parameters influxql#34 adds support for advanced bound parameters
- fix(influxql): don't expand large regex char sets influxql#30 is a fix for Some regexes will cause a stack overflow during evaluation #13929
Other than that, there's a go version update, optimization to use strings.Builder, and a simple fix to code comments - I wouldn't think these would have much user impact.
I suppose it would be good to have the CHANGELOG reflect these changes, although I'm not sure how that will work with the new changelog automation system.
Alternatively, I could just port the code out of InfluxQL in [this file](https://github.com/influxdata/influxql/blob/master/utils.go] into InfluxDB somewhere and use it that way. Do you have any strong feelings either way?
There was a problem hiding this comment.
I think we just take the influxql upgrade and call the version change out in the release notes.
…show tag values metaqueries (#22500) * feat: Add WITH KEY to show tag keys * fix: add tests for multi measurement tag value queries * chore: fix linter problems * chore: revert influxql changes to keep WITH KEY disabled * chore: add TODO for moving flux tests to flux repo Co-authored-by: Sam Arnold <sarnold@influxdata.com>
Closes #22385
Partial forward port of #20793
Partially closes #22499
This ports the changes made in #20793 to the TSDB's handling of predicate expressions, which fixes the bug with querying for tag values with a non-trivial predicate.
The InfluxQL changes are not being ported, since this would introduce a divergence between InfluxQL on 2.x and cloud. A feature request has been opened for adding support for this query to cloud, and depending on that decision the rest of the code could be ported. For now, only the changes to the TSDB are being ported to fix #22385.