Published client To Do

[tailhook]

Before publishing client crate on crates.io we need:

1. Ensure that error reporting is adequate and errors can be handled apropriately. Probably get rid of anyhow::Error. If even if we leave anyhow, we should ensure that it's easy to find out if error is recoverable, if it's network error, etc.
2. Figure out if it makes sense to move query methods into a trait (async traits are kinda complex, though)
3. Review list of dependencies, currently there are too much of them
   1. Also async-std should not require unstable feature
   2. Some of them, like whoami should be moved under a feature gate
4. Review public exports, hide unneeded things and restructure modules if needed
   1. Move Sequence under a feature gate (e.g. unstable)
5. Query arguments refactoring
   1. QueryArgs trait
   2. QueryArg trait
   3. QueryArg implementation for all basic types
   4. QueryArgs implementation for tuple and container types
   5. derive(QueryArgs)
6. Create a Datetime struct to represent edgedb's native datetime type Create a Datetime struct to represent edgedb's native datetime type #42
7. Implement most of the RFC1004
   1. Reconnecting connection
   2. Connection pool
   3. Transactions
   4. Retriable transactions
   5. Transaction configuration/Retry options
   6. Capabilities

These tasks can be postponed:

1. Look into supporting multiple runtimes Tokio support #49
2. Ensure that nothing is re-exported from edgedb-protocol, so that we can make breaking changes in latter without disturbing users
   1. Figure out what to do with Queryable trait
3. Take a look if we want to get rid of snafu for errors (used only internally)

Previous discussion in geldata/gel-cli#112, #30

In the meantime, anyone can use git version at their own risk (API will probably break every now and then)

[CodesInChaos]

The query functions taking a single arbitrary Value looks weird as well, since it's always a list of key-value pairs. I'd have expected some kind of map or named tuple there

[tailhook]

The query functions taking a single arbitrary Value looks weird as well, since it's always a list of key-value pairs. I'd have expected some kind of map or named tuple there

Well, it has to be either a tuple (i.e. a sequence) or a named tuple (i.e. a mapping) of variables. So accepting only a vector or only a mapping wouldn't work. Also a Vec<Value> is not very different to what we have now. And there aren't too many uses of Vec<T> because arguments are often of different types.

We can probably have a trait. But that trait is probably going to be just alias to Into<Value> (we may want to make more conversions supported, though).

[CodesInChaos]

If it's a tuple, how does the query access it? Something like $0 instead of $name? My preference would be using a smaller enum QueryArgs with only two options (tuple, named tuple) instead of Value here.

[tailhook]

If it's a tuple, how does the query access it? Something like $0 instead of $name? My preference would be using a smaller enum QueryArgs with only two options (tuple, named tuple) instead of Value here.

Yes. You can either write:

SELECT <str>$name

Or:

SELECT <str>$0

Smaller enum might work. But what individual elements are? Still Value?

[CodesInChaos]

I'd prefer something more trait based, so it works with static typing and not just Value.

Something along the lines of:

1. Introduce types encapsulating various Value variants, like NamedTuple and Tuple
2. Rename Queryable to something like QueryResult (or Deserialize, Decode)
   a) It's implemented for Value, the relevant primitve types and encapsulated Value variants
   b) It can be derived for structs
   c) It's implemented for standard tuples if the values are all QueryResult
3. Introduce a trait QueryArg (or Serialize, Encode)
   a) It's implemented for Value, the relevant primitve types and encapsulated Value variants
   b) It can be derived for structs
   c) It's implemented for standard tuples if the values are all QueryArg
4. Introduce a trait QueryArgs
   a) It's implemented for NamedTuple and Tuple
   b) It can be derived for structs
   c) It's implemented for standard tuples if the values are all QueryArg
   d) Add a macro to allow easy construction of a NamedTuple on the fly, e.g. named!(name=name(), password=pw())
5. Add a variant to Value which contains a Box<dyn QueryArg> so you can mix static and dynamic typing.

(Just a very rough idea, I'm sure that there are a bunch of details that don't make sense)

[CodesInChaos]

Ensure that nothing is re-exported from edgedb-protocol, so that we can make breaking changes in latter without disturbing users

I don't see how that's going to work. You can hide the message related parts. But edgedb-protocol also defines all the edgedb database types and the traits abstracting over them.

Probably get rid of anyhow::Error.

I agree. Anyhow is great for applications, like edgedb-cli, but I'd avoid it in libraries like edgedb-client.

[tailhook]

Ensure that nothing is re-exported from edgedb-protocol, so that we can make breaking changes in latter without disturbing users

I don't see how that's going to work. You can hide the message related parts. But edgedb-protocol also defines all the edgedb database types and the traits abstracting over them.

Well, yes Value is a problem. But if we go with some sort of your proposal above, we can allow statically typed queries without depending on Value, and statically typed queries are ones I expect to be at least 80% of Rust use cases (Value is mostly for REPL).

Will comment on the proposal of QueryArg above later.

[tailhook]

So the whole proposal looks sensible. Except few minor things:

1. Not sure about QueryResult name (especially if Combine RawCodec and Queryable? #40 fixed)
2. We don't support tuple arguments yet, so QueryArg for tuple is not needed yet
3. Box<dyn QueryArg> should be at least postponed. The primary purpose of Value is for returning value rather than for input. I think dynamic use cases might be covered well enough by using tuple of Box<dyn QueryArg> or struct wrapping that.

[CodesInChaos]

One problem with abstracting over Value and Queryable is context dependent (de-)serialization. Value is context dependent (especially on the deserialization side). So far Queryable is not.

Though there are some potential ambiguities even for Queryable, e.g. is the struct an Object or a NamedTuple, is the Vec a Set, an Array or even Bytes.

[tailhook]

One problem with abstracting over Value and Queryable is context dependent (de-)serialization. Value is context dependent (especially on the deserialization side). So far Queryable is not.

Though there are some potential ambiguities even for Queryable, e.g. is the struct an Object or a NamedTuple, is the Vec a Set, an Array or even Bytes.

Not sure I understand the exact problem you're trying to explain. With Queryable you're going to change the way it's decoded by:

#[derive(Queryable)]
#[edgedb(NamedTuple)]  // or Tuple
struct Struct {...}

Default is a shape (subset of the fields in an Object) because this is how we expect users to use edgeql. Set might be decoded into any kind of container (Vec, HashSet, BTreeSet, may be even maps, but I'm not sure).

Similar thing for encoding as QueryArgs. We only support certain kinds of arguments (no sets, only arrays, no tuples...)

Vec<u8> is probably more problematic. But we probably not going to implement QueryArg for u8 since there is no such type in edgedb. Minimum int is int16. So Vec<u8> is always bytes and doesn't conflict with Vec<T> where T:QueryArg as long as I understand.

[CodesInChaos]

With Queryable you're going to change the way it's decoded by

That's one way to resolve is ambiguity.

we probably not going to implement QueryArg for u8

This comes down to how you view the relationship between code and the database.

• The database schema is most important. The code is merely a way to conveniently access it and should mirror the database's representation as closely as possible. u8 should not implement Queryable since the database doesn't have a corresponding type.
• The code represents the data in a way that makes sense for it. The database is a place to store and query it. The driver should choose the best way to represent any given type in the code. u8 should implement Queryable and be mapped to i16.

In my experience the second approach worked very well with document databases and avoided the need for having a separate domain model. But I'm not certain how well it'd work with edgedb.

Set might be decoded into any kind of container

The problem with that is that both sets and arrays should be represented as containers in code, so there is still some ambiguity here. Fortunately arrays and sets have the same binary representation, so it's be possible to ignore this distinction.

There is one minor problem with that: Currently invalid headers produce different DecodeErrors for arrays and sets. You'd have to introduce a less specific error used when Queryable decodes an array/set.

One problem with abstracting over Value and Queryable is context dependent (de-)serialization. Value is context dependent (especially on the deserialization side). So far Queryable is not.

That problem still needs to be solved. Perhaps Queryable should be split into two traits, Decode and QueryResult where the decode function of QueryResult receives enough shape information for Value to work.

[1st1]

@tailhook

But we probably not going to implement QueryArg for u8 since there is no such type in edgedb.

We should add it, I don't think we omitted it deliberately.

The code represents the data in a way that makes sense for it. The database is a place to store and query it. The driver should choose the best way to represent any given type in the code. u8 should implement Queryable and be mapped to i16.

I'd prefer the driver to not chose anything on its own. Ideally types should be mapped 1 to 1.

[kaj]

While all of the things in the checklist here are worthwile and should be considered before an 1.0 release, I'm not convinced they should really be stoppers for a 0.1 (or 0.0.1) push to crates.io.

Even while explicitly not promising api stability, it might be a good idea to make it easy for people to test the things as they are, and, if nothing else, to reserve the name.

[tailhook]

I'll consider reserving name. But why publishing release that breaks every now and then is better than using git url for the same purpose? I know you can't publish on crates.io with git dependency, but that doesn't stop anyone from using edgedb in their own projects. Or you want to publish some tool that uses edgedb protocol?

[kaj]

Mainly I just think a series of rough prereleases would be nicer to work with than using the code straight from git. I'm not in a hurry, but it seems to me that actually fixing everything in the checklist will require years of development, and I think it would be nice to have something at crates.io earlier than that.

But that might just be me overestimating, so never mind.

Reserving the name might still be a good thing, though.

[tailhook]

Mainly I just think a series of rough prereleases would be nicer to work with than using the code straight from git.

Yes I agree. But they have associated cost and we need a plan for having a set of coherent releases that are better than arbitrary git commits. Currently we're busy getting Beta version of the database itself. Let's see if we can prioritize Rust bindings afterwards...

[dmgolembiowski]

Even while explicitly not promising api stability, it might be a good idea to make it easy for people to test the things as they are, and, if nothing else, to reserve the name.

@kaj is right; I believe there's an answer that satisfies both the community and the core developers: cargo features.

Alternatively, we can look to Debian; they use an interesting philosophy that might work here. Their package manager's "unstable" repository dissuades the masses from consuming non-perfected libraries. Who knows, maybe an edgedb-rs-preview crate could be the perfect compromise.

Eventually, this package should get phased out in favor of the edgedb-rs crate provided that it has @tailhook stamp of approval.

[tailhook]

Published 0.2.0 as the first step with no guarantees and a bunch of not implemented functionality.