Fix PartialEq behavior for UnionFields
#8937
Conversation
|
cc @alamb this should be fairly quick to review! |
| } | ||
| } | ||
|
|
||
| impl PartialEq for UnionFields { |
There was a problem hiding this comment.
I think this is a possible way of getting the desired behavior. In our project, we do have some mildly large unions (10-ish currently, but maybe more in the future) that occur very frequently and I am a bit anxious on how the quadratic equality check will perform in practice, as the equality of a data type is often used when comparing fields, schemas, and logical plans.
Another approach would be sorting the slice when constructing the instance. Maybe this would be an equally simple change with better performance as this already a private field.
Note that I have no idea how this will actually perform as I have no benchmarks. Just my 2 cents.
Otherwise, this looks like a good change!
2 participants
Which issue does this PR close?
PartialEqbehavior forUnionFields#8936Rationale for this change
This PR introduces a custom PartialEq impl for
UnionFieldsthat uses set semantics when determining equalityCurrently,
UnionFieldsderives PartailEq, but its internal representation is a list of(id, data type)tuples. As a result, equality is order-dependent, even thoughUnionFieldsis conceptually a set of unique(id, data type)pairs