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

`Queryable` not implemented for `Value`

Open d4h0 opened this issue 3 years ago • 3 comments

It seems, the Queryable trait isn't implemented for edgedb_protocol::value::Value.

Value would be quite valuable for debugging, logging, and so on.

d4h0 avatar Sep 27 '22 16:09 d4h0

Unfortunately it's not possible in the way trait is defined now. You can query a Value because query* methods are defined in terms of QueryResult trait instead of queryable, although that only works for top level result.

Do you need to put Value somewhere inside other derived Queryable?

tailhook avatar Sep 28 '22 10:09 tailhook

Do you need to put Value somewhere inside other derived Queryable?

Yes, I was trying to return (Vec<Value>, Vec<Value>, Vec<Value>) from EdgeDb. The Values represent different kinds of errors, that I only wanted to print to the terminal (so I wanted to avoid implementing structs for them).

After that didn't work, I just returned one Value, but ergonomics were so bad, that I changed my mind and implemented structs that derive Queryable, which is probably better anyway (and wasn't that that bad, in regard to ergonomics).

Another solution, I just realized, could have been to convert my error objects to JSON strings, and return that from EdgeDB.

Feel free to close the issue, if you don't plan to change the current implementation of the trait.

d4h0 avatar Sep 28 '22 10:09 d4h0

I think this issue can stay for the reference. Because I'm still not sure what the best way to handle this.

The core issue here is that:

  1. To unpack Value we need to know structure of the value that is encoded in descriptors and then decoded into Codecs.
  2. When we do statically typed unpacking we don't create a tree of Codecs and don't propagate it through the decoder code so Queryable is potentially faster way to decode data.
  3. And then there is QueryResult trait that handles codecs optionally.

Maybe Queryable can implement two modes, or maybe the overhead is not significant. Or maybe there are other solutions.

tailhook avatar Sep 28 '22 11:09 tailhook