-
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
Fix tag NEQ #2105
Fix tag NEQ #2105
Conversation
@@ -1188,7 +1193,6 @@ func TestSingleServer(t *testing.T) { | |||
nodes := createCombinedNodeCluster(t, testName, dir, 1, 8090, nil) | |||
|
|||
runTestsData(t, testName, nodes, "mydb", "myrp") | |||
runTest_rawDataReturnsInOrder(t, testName, nodes, "mydb", "myrp") |
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.
Obviously this will not be part of the final commit, just to speed up my testing.
Fix issue #2097. |
b154b0a
to
53d17a6
Compare
lgtm |
76e0a85
to
47025e9
Compare
expected: `{"results":[{}]}`, | ||
}, | ||
{ | ||
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.
I added an explicit reset-and-write here, since this test is subtle. I thought it was worth making it explicit.
@otoolep I made the same mistake in the |
524dfab
to
aef724a
Compare
If so, then return all series IDs which do not match. Fix for issue #2097
Series that do not have any tags are considered matching in the NEQREGEX case so the must be explicitly added.
+1 from @dgnorton |
if n.Op == influxql.NEQREGEX { | ||
ids = m.seriesIDs | ||
} | ||
|
||
for k := range tagVals { | ||
match := re.Val.MatchString(k) | ||
|
||
if (match && n.Op == influxql.EQREGEX) || (!match && n.Op == influxql.NEQREGEX) { |
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.
I think the || (!match && n.Op == influxql.NEQREGEX)
can be removed.
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.
OK, I will update the code, and if the tests pass, I will merge.
No description provided.