Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

Select series with label unset for != and !~ #228

Merged
merged 1 commit into from
Dec 21, 2017

Conversation

gouthamve
Copy link
Collaborator

@gouthamve gouthamve commented Dec 17, 2017

Fixes prometheus/prometheus#3575

Signed-off-by: Goutham Veeramachaneni cs14btech11014@iith.ac.in


This change is Reviewable

@gouthamve gouthamve requested a review from fabxc December 17, 2017 18:09
Copy link
Contributor

@fabxc fabxc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks great. Did you run any manual benchmarks? Would it make sense to add one to our test files since pulling in allPostings may have performance implications?

@@ -289,7 +289,7 @@ func (pb *Block) Delete(mint, maxt int64, ms ...labels.Matcher) error {
return ErrClosing
}

p, absent, err := PostingsForMatchers(pb.indexr, ms...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

db_test.go Outdated
testutil.Ok(t, err)
defer db.Close()

lbls := labels.Labels{labels.Label{Name: "labelname", Value: "labelvalue"}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

labels.FromStrings("labelname", "labelvalue") – those helpers were mostly added for tests.

db_test.go Outdated
defer q.Close()

ss, err := q.Select(labels.Not(labels.NewEqualMatcher("lname", "lvalue")))
testutil.Ok(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably cheap and easy to add a few other checks here as well.

Fixes prometheus/prometheus#3575

Signed-off-by: Goutham Veeramachaneni <cs14btech11014@iith.ac.in>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve
Copy link
Collaborator Author

Fixed all comments but did not run any benchmarks. Will do them later, sorry.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants