Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple databases in the same crate (compile-time macros) #916

Open
mehcode opened this issue Dec 21, 2020 · 23 comments · May be fixed by #3383
Open

Support multiple databases in the same crate (compile-time macros) #916

mehcode opened this issue Dec 21, 2020 · 23 comments · May be fixed by #3383

Comments

@mehcode
Copy link
Member

mehcode commented Dec 21, 2020

# .env
FOO_DATABASE_URL="postgres://..."

Option 1

Inspired from the log crate and log!(target: _).

let rows = sqlx::query!(database: "foo", "SELECT * FROM bar WHERE x = ?val", val = 10)
    .fetch_all(&mut conn)?;

Option 2

#[sqlx::database(url = "env:FOO_DATABASE_URL")]
mod foo;

// [...]

let rows = foo::query!("SELECT * FROM bar WHERE x = ?val", val = 10)
    .fetch_all(&mut conn)?;

Option 3

mod foo {
    sqlx::database!(url = "env:FOO_DATABASE_URL");
}

// [...]

let rows = foo::query!("SELECT * FROM bar WHERE x = ?val", val = 10)
    .fetch_all(&mut conn)?;

That's all I can think of at the moment. I prefer Option 2.

This is a solution to #121 and #224.

Thoughts?

@abonander
Copy link
Collaborator

abonander commented Dec 21, 2020

From a UX perspective, 3 is the friendliest to IDEs because it can be done with macro_rules! macros (can even be built on top of 1). Number 2 would not work at all with code-completion in IDEA. It's also non-local to the module file which I don't really like, although it could be the inner form #![sqlx::database(...)] but that would get lost above the imports in the kind of module that's typical of the projects we've been working on.

@mehcode
Copy link
Member Author

mehcode commented Dec 21, 2020

Option 4

sqlx::database!(name = "foo", url = "env:FOO_DATABASE_URL");

// [...]

let rows = foo::query!("SELECT * FROM bar WHERE x = ?val", val = 10)
    .fetch_all(&mut conn)?;

To keep it 1-line (if that's important), we could inline the module creation within "Option 3".

@abonander
Copy link
Collaborator

I would think name = should be to say "use all the parameters from DATABASE_URL but change the database name to this".

What about

sqlx::database!(mod foo, name = "bar_test_db")

which declares a module foo and redefines the query macros to talk to bar_test_db with the same authority and params as DATABASE_URL.

@mehcode
Copy link
Member Author

mehcode commented Dec 21, 2020

I like the idea of name being "use the same URL but change the name to this".

I don't like !(mod foo, as syntax.

I'd probably prefer the 3-line mod foo { ... } version over that.

# .env
DATABASE_URL="postgres://postgres@localhost/foo"
OTHER_DATABASE_URL="postgres://postgres@other/foo"
mod bar { sqlx::database!(name = "bar"); } // url is "postgres://postgres@localhost/bar"
mod other_foo { sqlx::database!(url = "env:OTHER_DATABASE_URL"); } // url is "postgres://postgres@other/foo"

@abonander
Copy link
Collaborator

abonander commented Dec 21, 2020

name is really ambiguous anyway.

What about

sqlx::database!(mod = "foo", db = "bar")

@andrewwhitehead
Copy link
Contributor

What else is it going to produce besides a mod, presumably pub(crate)?

sqlx::database!(foo, url = "env:FOO_DATABASE_URL");

Although if the user wants to put additional items in that module or adjust the visibility, the above macro could delegate to another one with both exposed:

mod foo { sqlx::database_impl!(url = "env:FOO_DATABASE_URL"); }

@mehcode
Copy link
Member Author

mehcode commented Dec 21, 2020

What else is it going to produce besides a mod, presumably pub(crate)?

That's a really good reason for me to not generate the rust module at all. I hadn't considered visibility.

@abonander
Copy link
Collaborator

Yeah, that could get messy.

@abonander
Copy link
Collaborator

@mehcode I just realized we were already talking about doing this a long time ago, potentially with allowing other configuration to the macros such as specifying type overrides #121

We also have talked about a sqlx.toml but I'm still somewhat of the persuasion that we shouldn't have an external config file that changes the behavior of the macros as that's a non-local effect.

@abonander
Copy link
Collaborator

In talking about it, @mehcode and I settled on sqlx::macros!(), short and sweet.

@abonander
Copy link
Collaborator

abonander commented Feb 3, 2021

Unfortunately, the whole idea with using sqlx::macros!() in two different modules so you could do, e.g. foo::query!() and bar::query!() isn't gonna work with macro_rules! as it exists in this day and age.

Even in the 2018 edition you still can't invoke a macro in the same crate via its fully-qualified path; you have to use #[macro_use] to import all macros from a submodule into the parent module and all child modules following the annotated submodule in declaration order. Presumably declarative macros 2.0 would let this work as expected as it actually participates in normal item resolution but I have no idea when that's going to be stabilized.

There's still some utility with sqlx::macros!() but using it with two separate databases at once would require either invoking it in separate crates or making sqlx::macros!() a proc-macro so it can append a user-defined prefix to the macro names, which as mentioned above would kill autocomplete for the macros at least with IntelliJ-Rust.

We could make it work like this, though:

sqlx::macros!(
    db_url[foo] = "env::DATABASE_FOO",
    db_url[bar] = "env::DATABASE_BAR",
    types[bar] = {
        "BAZ": crate::model::Baz,
    },
    // top-level option which applies to both `foo` and `bar`:
    types = {
        "CITEXT": String,
    }
);

// this matches how arguments are passed to the macros internally but can be changed
query!(db = foo, "select * from ...")

I'm thinking any configuration options we plan to add, we can have them optionally accept a database name as subscript, and then they only apply to that database. Otherwise, they apply to all databases.

It could also look like this, maybe (again, with top-level options applying to all databases):

sqlx::macros! (
    db.foo = {
        url: "env::DATABASE_FOO",
    },
    db.bar = {
        url: "env::DATABASE_BAR",
        types: {
            "BAZ": crate::model::Baz,
        }
    },
    types = {
        "CITEXT": String,

    }
);

Or combine both and make the subscript db[foo] = { ... }.

@abonander
Copy link
Collaborator

However, I'm also recalling some issues with macros-defining-macros where you want to use parameters from the outer macro invocation in the inner macro... things get weird.

Maybe it's time to dust off the sqlx.toml proposal.

@jplatte
Copy link
Contributor

jplatte commented Feb 3, 2021

Even in the 2018 edition you still can't invoke a macro in the same crate via its fully-qualified path; you have to use #[macro_use] to import all macros from a submodule into the parent module and all child modules following the annotated submodule in declaration order. Presumably declarative macros 2.0 would let this work as expected as it actually participates in normal item resolution but I have no idea when that's going to be stabilized.

Actually there is some movement towards integrating macro_rules! with regular import resolution; see rust-lang/rust#78166.

Wanted to link to that earlier but apparently it's virtually impossible to find with regular search engines, I only found it again now because I got a notification about a new comment.

@abonander
Copy link
Collaborator

It's been put to an RFC now so it's still going to take a while to get merged and then stabilized, but it's nice to see that it's possible. It sounds like they want to make it part of the 2021 Edition though.

As for this:

However, I'm also recalling some issues with macros-defining-macros where you want to use parameters from the outer macro invocation in the inner macro... things get weird.

Apparently it just works now so I'm not sure what funkiness I was remembering: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=4de9983692e5894f174caf52b6c49191

@mehcode
Copy link
Member Author

mehcode commented Feb 4, 2021

I think to reduce the syntax surface area I would be much more in favor of:

// all do the same, just bikeshedding
sqlx::macros!(foo, url = "env:DATABASE_URL");
sqlx::macros!(name: foo, url = "env:DATABASE_URL");
sqlx::macros!(prefix: foo, url = "env:DATABASE_URL");
sqlx::macros!(prefix = foo, url = "env:DATABASE_URL");

and used as:

foo_query!("SELECT ...");
foo_query_as!(Bar, "SELECT ...");

Yes this requires the macros to be in lib.rs or #[macro_use]'d near the top of the file, but I don't like a limitation like that will matter much to people that want this feature.

@mehcode mehcode removed this from the 0.5 milestone Mar 16, 2021
@Freyskeyd
Copy link
Contributor

Hello

I'm facing the issue where I've multiple DB connections in the same crate. I would like to work on a solution, can we fixe one of the propose option as a base to iterate?

@abonander
Copy link
Collaborator

Sadly the pub_macro_rules feature was removed in rust-lang/rust#83713 so we're likely back to just supporting prefixes for the new macros as depicted in @mehcode's comment above.

@aaronmussig
Copy link

While this may not be a proper solution, it's a workaround that works in my case (as I have no duplicate table names, and my database schemas are not updated often), so maybe it will be of use to someone else.

Essentially I created a new database (C) that contains the schema of tables in databases A and B. The connection string to database C is then set in the .env file. Now all query! calls will check against the schema of C.

It's then up to me to make sure that the correct database worker pool (i.e. database A or B) is used when calling query!.

@nate-sys
Copy link

nate-sys commented Feb 8, 2024

I'm having this issue as well. wondering if there's any progress

@jssblck
Copy link

jssblck commented May 24, 2024

I'm working with multiple databases at the same time in a codebase, and this is a real issue- it's so unfortunate to have to give up 90% of what makes SQLx great to use (compile time queries) due to this.

Is this something you're looking for community help on?

@docwilco
Copy link

_with variants of the various query macros could be a thing. Would it be with name of the env var containing the URL? Or just the URL itself? Latter might be a bit more flexible for people who don't want to use a .env file.

@abonander
Copy link
Collaborator

The current plan is to use the sqlx.toml format I'm working on in #3383.

There will be a new proc-macro to define a new set of query!() and migrate!() macros at the call site:

sqlx::macros_with_config!("sqlx.foo.toml");

This will look for sqlx.foo.toml relative to the crate root and use it to configure the new macros, overriding any existing sqlx.toml config. The TOML file can be used to override the DATABASE_URL var, set default type overrides, change the table name for tracking migrations, etc.

Under the hood, this will just pass the file path to sqlx_macros::expand_query!(), resolved relative to the CARGO_MANIFEST_DIR for the crate that invoked macros_with_config!.

Additionally, it can support some control arguments:

sqlx::macros_with_config!(
    "sqlx.foo.toml",

    // Add a prefix to the macro names
    prefix = "foo_" // (`foo_query!()`, `foo_migrate!()`, etc.)

    // Add attributes to the generated macros 
    attr(macro_export), // (`#[macro_export]`)
    attr(cfg(feature = "foo")) // (`#[cfg(feature = "foo")]`),'

    // Any others we might want?
);

@AustinCase
Copy link

@abonander very excited about seeing some activity here. We were about to fork and see if we could get this working, obviously hoping to contribute back once we did -- great timing.

I was headed down the sqlx.toml path as well as I was documenting our plan -- that seemed to be the most straight forward and rust-analyzer friendly path.

Any help / insight myself or our team can provide to get this across the line, let us know!

@abonander abonander linked a pull request Sep 20, 2024 that will close this issue
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants