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

feat: make sqlx work with multiple different databases in the same crate #3397

Closed

Conversation

Aderinom
Copy link

@Aderinom Aderinom commented Aug 2, 2024

This PR provides a working solution for allowing sqlx users to utilize multiple different databases during compile/prepare steps.

The solutions is loosely based on the proposals around this topics in the "solved issues"
Three changes have been implemented:

  1. The expand_query! procmacro now takes an additional optional key db_url_env - if this key was passed the given enviroment variable will be used to find the DB URL instead of the default one.
  2. On sqlx prepare - all queries using a custom db_url_env will save their queries in a subfolder to the offline path.
    (e.g. db_url_env = DATABASE_URL_DB_A -> query save path will be .../.sqlx/DATABASE_URL_DB_A/sqlx-query...)
    This makes sure that the same queries on different databases never collide.
  3. A new proc macro "database_macros!" was added to generate "query!()" macros for a specific custom env

The new macro can be used as follows:

#[allow(unused_macros)]
pub mod some_db {
    sqlx::sqlx_macros::database_macros!(env = "DATABASE_URL_SOME_DB");
}

fn test() {
  some_db::query!("Select a from b").fetch_all(...);
}

Code completion using rust analyzer with VsCode and Neovim works without a problem.

I am a little bit unhappy with the macro itself, as the entire macro has a full copy of all the query macros.
see sqlx-macros-core/src/database_macro/expand.rs
If anyone has a suggestion here I'd love to hear it.

One alternative Idea I had was to also generate the macros in src/macros/mod.rs with this macro as well.
However I am not sure how well this will work with docs.

Does your PR solve an issue?

fixes #916
fixes #224
fixes #121

@abonander
Copy link
Collaborator

I've actually already started working on this, but going in a slightly different direction.

The idea is based on #3383; instead of passing configuration options to the macro directly, you'd give it the path to a different TOML file, and that would let you override all kinds of things. It would also configure the migrate!() macro.

pub mod some_db {
    sqlx::sqlx_macros::database_macros!(env = "DATABASE_URL_SOME_DB");
}

fn test() {
  some_db::query!("Select a from b").fetch_all(...);
}

This unfortunately wouldn't work within the same crate. Macros only use path-based resolution when they're defined in a different crate. Within the same crate, you have to put #[macro_use] on the module that defines them, and it depends on expansion order. This isn't very well explained in official documentation, but it is covered briefly in the Little Book of Rust Macros.

@loganbnielsen
Copy link

This unfortunately wouldn't work within the same crate.

Which part wouldn't work within the same crate?

For the tomls would a different sqlx toml be dedicated to each DB configuration?

Would loading a toml disable using sqlx env variables so that the configs aren't coming from multiple sources?

Might be a bit heavy handed to load a toml when only a URL change is needed... Could both options be supported?

Personally I've only needed URL overrides so far

@abonander
Copy link
Collaborator

Which part wouldn't work within the same crate?

You cannot refer to a macro by path within the same crate: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=dee33ddf878f6ec5cff9baa725f7781c

If you mark the macro with #[macro_export] then it's exported at the crate root and can be referenced as such: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=73036b2707ee310442d919270e0b8fad

That macro can be referenced by path, but only by other crates. This was a quality-of-life improvement that came with the 2018 edition.

If you put #[macro_use] on the declaring module, the macro is available globally within the crate, but only for modules later in the declaration order: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9f74e71676aad5ab138c653e46780b54

Yes, it's incredibly confusing. The Rust macro system is a bunch of hacks piled on top of more hacks. Macros 2.0 was supposed to fix that, but that's been stalled since forever.

For the tomls would a different sqlx toml be dedicated to each DB configuration?

Yes.

Would loading a toml disable using sqlx env variables so that the configs aren't coming from multiple sources?

No. There's only a couple env vars of consequence, DATABASE_URL and SQLX_OFFLINE, and both of those have to do with the current environment which is why it makes more sense for them to be environment variables. The .env file is just a convenience, it's not a config format.

Might be a bit heavy handed to load a toml when only a URL change is needed... Could both options be supported?

I thought about this, but when you have multiple ways to do something it just ends up confusing the user and being a bigger maintenance burden. It's better if everything just goes in the one file, then you always know where to look. Chances are, you might end up needing to tweak a few other things later anyway.

@loganbnielsen
Copy link

Makes sense, thank you for walking me through that!

@Aderinom
Copy link
Author

Aderinom commented Aug 5, 2024

Instead of the #[macro_use] for each macro I have added a pub (crate) use macro
This does not make the macro global, seems to not care about the declaration order, and still allows you to use normal paths to call it like any other item:

(edit: Ah, just now saw that this only works from 2018 upwards.)

mod after_foo {
    pub fn uses_bar() {
        super::foo::bar!();
    }
}

mod foo {
    macro_rules! bar {
        () => {
            println!("Hello, world!")
        };
    }
    pub(crate) use bar;
}

fn main() {
    foo::bar!();
    after_foo::uses_bar();
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f6e17f11d6fae24f74f344c174622ff0

I'm not seeing any unexpected behavior from this in the project were we are using the forked branch atm.

Happy to hear that you are working on the TOML solution though.
Is there anything one could do to help out there?

@AustinCase
Copy link

AustinCase commented Aug 17, 2024

Instead of the #[macro_use] for each macro I have added a pub (crate) use macro This does not make the macro global, seems to not care about the declaration order, and still allows you to use normal paths to call it like any other item:

(edit: Ah, just now saw that this only works from 2018 upwards.)

mod after_foo {
    pub fn uses_bar() {
        super::foo::bar!();
    }
}

mod foo {
    macro_rules! bar {
        () => {
            println!("Hello, world!")
        };
    }
    pub(crate) use bar;
}

fn main() {
    foo::bar!();
    after_foo::uses_bar();
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f6e17f11d6fae24f74f344c174622ff0

I'm not seeing any unexpected behavior from this in the project were we are using the forked branch atm.

Happy to hear that you are working on the TOML solution though. Is there anything one could do to help out there?

Thank you for this fork 🙏🏼 just tried it out in a quick dummy project and seems to work as expected. Might implement this in the short term... depending on the status of sqlx.toml

I also have a fork that starts creating the sqlx.toml @abonander -- it isn't very far along and seeing your PR I am hesitant to keep investing in it. Is it a priority for you? Would you be open to the outside contribution on this?

@abonander
Copy link
Collaborator

Closing in favor of #3383, which I'm working on as fast as I can.

I appreciate the effort here, but it's unfortunately using an old solution that's been superceded by sqlx.toml.

@abonander abonander closed this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants