rust-client icon indicating copy to clipboard operation
rust-client copied to clipboard

Range type should have the ability to hold both f64 and i64 fields

Open inejc opened this issue 2 years ago • 1 comments

Hi, according to the docs, it is possible to use range filtering on both float and integer payloads but the Range client type can only hold f64 optional fields, i.e.: https://github.com/qdrant/rust-client/blob/cd7ee0f5946bdf4d5c49de438230f8f9d337a6fc/src/qdrant.rs#L2931-L2940

inejc avatar Jul 12 '23 12:07 inejc

Hey, I've taken a look at this and I'd be willing to work on this if the maintainers deem it worthwhile. From what I can see it will require changes in at least the qdrant engine as well as the rust client.

So here's what I understand (at least I think I do, but I'll gladly take hints):

What you showed is the protobuf generated Rust code which comes from the protobuf definitions. The Range structure is used in the Condition structure, which is used in the SearchPoints structure that is passed to the search points member function of the QdrantClient. We cannot unilaterally change the layout of the Range struct because its part of the data that gets passed as a remote procedure call to the qdrant engine. From what I saw in the qdrant engine, there is a conversion layer that converts this range type to segment::types::Range. This is the type that is actually used by the engine but it also only contains Option<FloatPayloadType> fields (with FloatPayloadType = f64). So one would have to extend the Fields of this type to be able to hold either i64 or f64 which is definitely possible but will touch a lot of other code.

Hence, I would only want to do this if a maintainer is fine with that. Plus I'd like to know if I missed something and everything is much simpler after all :grin:

geo-ant avatar Jul 26 '23 20:07 geo-ant