-
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
Also include NEQ in filters #2104
Conversation
@@ -588,6 +588,21 @@ func runTestsData(t *testing.T, testName string, nodes Cluster, database, retent | |||
expected: `{"results":[{"series":[{"name":"cpu","tags":{"region":"us-east"},"columns":["time","mean"],"values":[["1970-01-01T00:00:00Z",15]]},{"name":"cpu","tags":{"region":"us-west"},"columns":["time","mean"],"values":[["1970-01-01T00:00:00Z",30]]}]}]}`, | |||
}, | |||
|
|||
// WHERE tag queries | |||
{ | |||
reset: true, |
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.
Added some =
unit tests for tags. These always worked, but I wanted some tests.
if n.Op == influxql.EQ || n.Op == influxql.LT || n.Op == influxql.LTE || n.Op == influxql.GT || | ||
n.Op == influxql.GTE || n.Op == influxql.EQREGEX || n.Op == influxql.NEQREGEX { | ||
switch n.Op { | ||
case influxql.EQ, influxql.LT, influxql.LTE, influxql.GT, influxql.GTE, influxql.EQREGEX, influxql.NEQREGEX, influxql.NEQ: |
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.
The fix is here -- add influxql.NEQ
. I also changed to the code to use switch
-- I think it's easier to read now.
@@ -475,6 +475,8 @@ func (m *Measurement) walkWhereForSeriesIds(expr influxql.Expr, filters map[uint | |||
|
|||
// finally return the ids and say that we should include them | |||
return ids, true, nil | |||
default: |
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.
This may not be the right thing to do, but it would have flagged the bug immediately.
74c0175
to
0757bf3
Compare
This fixes != for field value comparisons.
LGTM, 🚢. Since you were adding test cases for queries against tags, it wouldn't hurt to add some that test against multiple tags using |
Let me get back to that -- I'm working on tags in #2105, and will add more tests there. |
This fixes != for field value comparisons.