Skip to content

LabelPair PartialOrd inconsistent with PartialEq #513

@rtimush

Description

@rtimush

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions