search

apache / iceberg-rust

Apache Iceberg
https://rust.iceberg.apache.org/
Apache License 2.0
469 stars 95 forks source link

bug: `PrimitiveLiteral` and `Literal` should not be `Ord`. #378

Closed liurenjie1024 closed 1 month ago

liurenjie1024 commented 1 month ago

In rust, Ord means total order, while PartiarOrd mean partital order. Currently we have derived Ord for both PrimitiveLiteral and Literal, which is incorrect. How could we compare Struct with Map, or Date with String? We should do following changes:

  1. Remove PartialOrd, Ord for both Literal.
  2. Remove Ord for PrimitiveLiteral. I think we should also remove PartialOrd for PrimitiveLiteral, which misses type information for decimal.
  3. Implement PartialOrd for Datum.
liurenjie1024 commented 1 month ago

I think this is blocking prs with filter related, such as #363 and #367

liurenjie1024 commented 1 month ago

cc @sdd @marvinlanhenke @Xuanwo @Fokko