protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Proposal: add proto.Compare

Open ericchiang opened this issue 5 months ago • 7 comments

Is your feature request related to a problem? Please describe.

This is a feature request to provide a method similar to proto.Equal that works with Go's slices and cmp packages. This comes up often when attempting to sort two lists of protos and determining a diff.

Describe the solution you'd like

// Compare provides an ordering of two message of the same type, returning -1 if
// x is "less than" y, 0 if x is equal to y, or +1 if x is "greater than" y. This is intended
// for use with [slices.SortFunc] to compare two slices within the same program.
//
// The result of Compare is consistent only within the same binary, and follows rules
// similar to [MarshalOptions.Deterministic]. Wither a message is "less than" or
// "greater than" another message is arbitrary and will change over time.
//
// This method uses the same rules as [Equal] to determine equality.
func Compare(x, y Message) int

Describe alternatives you've considered

A current alternative is to encode each method with MarshalOptions.Deterministic and use that as a source of ordering.

Additional context

I wrote a version of this internally at Google a while back (called "protosort"), and it turned out that it was common enough that someone else had written the same package as well. It'd be great if this was just supported out of the box by the proto package.

ericchiang avatar Jul 08 '25 18:07 ericchiang

We have proto.Equal, having Compare as well seems like a straightforward extension and a reasonable feature request. (I might, of course, be missing something that makes it less straightforward than it seems.)

neild avatar Jul 08 '25 18:07 neild

Thanks Damien,

Looking into the implementation, Equal is powered by protoreflect.Value.Equal. Would it be okay in principle to add a Compare method to that type as well and switch Equal to use Compare under the hood (baring performance issues)? If so that should be straightforward to add.

ericchiang avatar Jul 08 '25 18:07 ericchiang

Yes, I think we'd need a protoreflect.Value.Compare as well.

(Note that I'm just an emeritus maintainer making a drive-by comment, current maintainers will be the ones to decide whether we should do this or not.)

neild avatar Jul 08 '25 19:07 neild

I went ahead and sent https://go-review.googlesource.com/c/protobuf/+/686795

ericchiang avatar Jul 09 '25 07:07 ericchiang

🤔 I’m not sure how I feel about defining any sort of canonical ordering of protobufs.

Although, after reading the CR, it does explicitly note that there’s no guarantee of continuity of comparisons, but I think this could be installing an new footgun for people. Some people will inevitably fail to read the documentation, or not read it enough, and expect this to be a consistent sorting between recompiles, and then build some code in that relies on that ordering and then when a new recompile gets them a new ordering, they’re confused why suddenly their from-persistence slice of protobufs is no longer sorted.

This was kind of the purpose of non-deterministic marshalling in the first place. To make blindly marshaling a protobuf into JSON and checking it against a golden value fail early and fail loudly.

puellanivis avatar Jul 10 '25 17:07 puellanivis

I inherited a system a while back that depended on the determinism of the old marshalling, defiantly not something that I want to encourage...

ericchiang avatar Jul 10 '25 20:07 ericchiang

Gah github, I accidentally closed this ~and don't have permissions to reopen...~ (edit nvm, apparently I do)

I inherited a system a while back that depended on the determinism of the old marshalling, defiantly not something that I want to encourage. I have used the proto package's deterministic marshaling in Spark jobs to get an encoding that is the same between two binaries running in the pipeline though.

I've written a huge amount of code where there's two list of protos and I need to get a diff to determine what needs updating in a database. I ended up using Google's internal protosort package a ton, and it might be worth seeing if people how people are using (or abusing) that internally for anyone with access (I obviously can't anymore 😅 )?

Maybe naming the function something would help?

func CompareUnstable(x, y proto.Message) int

And/or perhaps introduce some randomness at startup to reverse the order 50% of the time for a given program? Similar to what the Go team wished they'd done with sort:

https://github.com/golang/go/issues/13884

ericchiang avatar Jul 10 '25 20:07 ericchiang