Dushistov / couchbase-lite-rust

Lightweight, embedded, syncable NoSQL database engine wrapper for Rust
Apache License 2.0
25 stars 10 forks source link

use serde fleece to decode query params #67

Closed Dushistov closed 2 years ago

Dushistov commented 2 years ago

Close #66 .

agg23 commented 2 years ago

Sorry for the delay, but I would like to see a complete solution in regards to parsing the query result enumerator. #66 has an entire separate serde Deserializer due to the need to parse individual result columns into the destination type. Your solution gets us halfway, but I would argue that the "hard" part is figuring out how to set up serde to accomplish this.

If you know a nicer way to do this, I would love to see it, particularly if you can get rid of all of the excess deserialize_* methods.

Dushistov commented 2 years ago

Not sure what you mean.

Do you mean that it is hard to user to write something like this, and this should be in library?

            let mut vec = Vec::<(T1, T2, ..)>::with_capacity(100);
            while let Some(item) = iter.next()? {
                let val1: T1 = item.get_checked_serde(0)?;
                let val2: T2 = item.get_checked_serde(1)?; 
                vec.push((val1, val2));
            }

?

agg23 commented 2 years ago

It is hard to write a generic version of the above code, yes. As shown in my example, serde can take care of the "dynamic" (really per query) number of columns, whereas to replicate the functionality without serde, you would need to write a custom derive proc-macro (or maybe you could write an unsafe macro_rules! that wraps the query itself).

Dushistov commented 2 years ago

So the general idea to write:

while let Some(item) = query_iter.next()? {
    let (p1, p2, ...): (T1, T2, ...) = deserialize_with_serde(query_iter)?;

maybe you could write an unsafe macro_rules! that wraps the query itself

Why it should be unsafe? With current code base macro_rules! should just generate from T1, T2,.. list the code like this:

(item.get_checked_serde::<T1>(0)?,
  item.get_checked_serde::<T2>(1)?, 
  ...)

this should not contain any "unsafe" code.

agg23 commented 2 years ago

You're right about the unsafe comment; for some reason I was thinking that you'd have to forcibly cast, but that's already taken care of by get_checked.

Ideally you'd just write something like:

let results: Vec<T> = from_query::<T>(query.run());

(or even better, with .into()) though that's nearly equivalent to your

while let Some(item) = query_iter.next()? {
    let (p1, p2, ...): (T1, T2, ...) = deserialize_with_serde(item)?;

I personally prefer to be more direct about how to get from query to results, hence why I like having a wrapper that provides the mapping from columns to a single output struct.