apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
6.3k stars 1.19k forks source link

`PartialOrd` for structs with incomparable fields #12537

Open ngli-me opened 1 month ago

ngli-me commented 1 month ago

Is your feature request related to a problem or challenge?

What requires this manual derivation? Is it some other extension trait would need to also require PartialOrd?

I am thinking it would be really nice to file a ticket tracking what it would take to use a derived PartialOrd to make this code more maintainable

_Originally posted by @alamb in https://github.com/apache/datafusion/pull/12481#discussion_r1761693610_

12481 gives a possible implementation for PartialOrd, using a mix of manual implementations and derivations. The main blocker on using only derivations is DFSchemaRef, which is an incomparable type, along with the associated Schema, and minorly HashMap.

Describe the solution you'd like

There are a couple of possible solutions. One simple one might be to just give a manual implementation of PartialOrd for DFSchemaRef that just returns None, allowing for types using it to derive PartialOrd. However, this would still leave a gap for the other datatypes used.

Describe alternatives you've considered

Another solution would be to use something like the derivative crate, but that means another attribute for the struct, and additional attributes for the fields being excluded.

The current alternate solution being used for structs with >3 fields in #12481 is to have a inner struct for the PartialOrd manual implementation, that excludes in incomparable fields. This is easier to maintain, but also has a few drawbacks.

Additional context

Relevant: #8932.

alamb commented 1 month ago

Another alternative might be a manual implementation PartialOrd for DFSchemaRef perhaps based on field names or something

alamb commented 1 month ago

Another solution would be to use something like the derivative crate, but that means another attribute for the struct, and additional attributes for the fields being excluded.

It also means another dependency which would be nice to avoid if possible