Skip to content
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

LabelPair PartialOrd inconsistent with PartialEq #513

Open
rtimush opened this issue Jan 26, 2024 · 0 comments
Open

LabelPair PartialOrd inconsistent with PartialEq #513

rtimush opened this issue Jan 26, 2024 · 0 comments

Comments

@rtimush
Copy link

rtimush commented Jan 26, 2024

Describe the bug
PartialOrd is implemented manually for prometheus::proto::LabelPair and it only compares the name. However, PartialEq is derived, so it checks all fields. This violates PartialOrd requirements (a == b if and only if partial_cmp(a, b) == Some(Equal)) and may cause issues if LabelPair is used in standard containers.

To Reproduce

let mut a = LabelPair::new();
a.set_name("name".to_owned());
a.set_value("a".to_owned());

let mut b = LabelPair::new();
b.set_name("name".to_owned());
b.set_value("b".to_owned());

assert!(a.cmp(&b) == Ordering::Equal);
assert_eq!(a, b) // fails

Expected behavior
Either PartialEq should be implemented manually to only compare the name field for consistency with PartialOrd, or PartialOrd should compare all fields. It's also worth mentioning in the docs if only name constitutes the equality, as it can be quite surprising.

Additional context
I had a bug because tried to put LabelPair into a BTreeSet.

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

No branches or pull requests

1 participant