Backend agnosticism.

[Virgiel]

We currently only support tokio-postgres, but the syntax and a good part of the library logic are not specific to async PostgresSQL. I would like to use this library with non-async postgres and SQLite via rusqlite.
As there will be plenty of breaking changes in version 0.8, this would be a good time to add breaking changes now allowing support for other backend in future versions.

The Query and GenericClient type should be backend specific: cornucopia_client::postgres::Query and cornucopia_client::postgres::GenericClient. Backend support could use crates features when they are not compatible (Postgres vs SQLite), but we need to find a way to support both sync and async. Maybe always generate both with async by default and a separate module for sync: cornucopia_client::postgres::sync::Query and cornucopia_client::postgres::sync::GenericClient.

[Virgiel]

We should also replace rustfmt command calls with prettyplease. dtolnay created this library precisely to format generated code.

Currently, we already depend on the presence of docker, podman or a running database to generate code. This is a necessity for Postgres but if we support other backends that can be embedded, it would be interesting not to depend on a predefined binary in the environment.

Using prettyplease and an embedded SQLite database would allow to generate code in the build script using cornucopia as library.

[LouisGariepy]

@Virgiel Thank you for your suggestion 😄.

(I created a separate issue #54 for prettyplease)

I'm not sure about Cornucopia being driver-agnostic, but I'd be interested in supporting 1) the synchronous postgres driver and 2) an embedded driver, probably Rusqlite, as you mention.

The reason is that tokio-postgres (or postgres) does a lot of heavylifting for us with respect to type registration, which is probably the most complex part of the whole crate. Notwithstanding other particularities, the driver should give us at least as much type info as tokio-postgres does, otherwise we have to maintain a bunch of complex, driver-specific code.

Synchronous postgres should be painless to add, and my hope is that Rusqlite will be simple enough with regards to typing to not cause any problems. For anything beyond that, I'd potentially accept a PR, but it depends on how it affects the crate's maintainability.

[Virgiel]

Synchronous postgres is already good to have!

[Virgiel]

We could make both sync and async Postgres two features that can be combined. We can reuse all the generated types and only need a sync query implementation.

Since postgres is based on tokio-postgres, if our tests pass with postgres, they should pass with tokio-postgres. Therefore, we should be able to remove async from codegen.

My question is how do structure the code and how do we adapt the cli for this?

For the module hierarchy I propose this:

pub mod types {
    pub mod db {
        // database types (composites types)
    }
    pub mod module_1 {
        // module_1 types (params and rows)
    }
    // more modules
}
pub mod queries {
    pub mod module_1 {
        pub mod async { // (if postgres features)
            // module_1 async queries and function 
        }
        pub mod sync { // (if tokio-postgres features)
            // module_1 sync queries and functions
        }
    }
    // more modules
}

[LouisGariepy]

Agreed. I'm curious to know in which case would it be useful to have both sync and async version in the same application though. As you say it'd be easy to make sync and async additive so I'm on board, i'm just curious 😄

Can't use async as a module name though, and I'm not too keen on using a raw identifier.

For the cli I suggest going with a straightforward --sync and --async flags (potentially using both). If none are used, it'd default to --async only, as the majority of apps using Cornucopia are web-servers, and I think it'd be more convenient for users that way.

[LouisGariepy]

@Virgiel Looking at how Rusqlite does type discovery (the decl_type() method at https://docs.rs/rusqlite/latest/rusqlite/struct.Column.html#impl), and it notes that expressions will result in None. This seems to me like a limitation which would prevent Cornucopia to work (no access to type info), but maybe I'm wrong. Maybe we error out if such a column is found. I'm really not sure.

I'll wait for your comments 😄

[Virgiel]

I underestimated how weakly typed SQLite is :/
We could return a raw ValueRef and use generic ToSql for parameters. We lose a lot of typing, which is sad, but we'd still gain some rust glue, especially the naming of parameters and returned columns. Using genericity over From and Into, the user could provide their own typed version of the returned parameters and column structures.
This is definitely a long shoot. I will experiment with the idea after the next release !