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 running macros in "decoupled" mode #60

Closed
3 of 5 tasks
abonander opened this issue Jan 15, 2020 · 5 comments
Closed
3 of 5 tasks

Support running macros in "decoupled" mode #60

abonander opened this issue Jan 15, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@abonander
Copy link
Collaborator

abonander commented Jan 15, 2020

This was a popular request in the original Reddit announcement: https://www.reddit.com/r/rust/comments/egpw7g/announcing_sqlx_a_fully_asynchronous_pure_rust/fc8ldpw/?context=1000

The idea is to allow expansion of query!(), et al, without having to connect directly to the database by running the PREPAREs for all queries in a separate step and caching the result.

The solution in my head looks something like this:

  • We create a Cargo subcommand, cargo-sqlx, which links sqlx-core internally.
  • The user runs cargo sqlx prepare in their project which invokes cargo check with QUERY_PREPARE_OUT=$OUT_DIR/sqlx set in the environment
    • By being a subcommand we can know which cargo they're using so we build with the right toolchain, e.g. if the user runs cargo +nightly sqlx prepare when their default toolchain is stable
  • When query!() and friends see QUERY_PREPARE_OUT in the environment they don't perform normal expansion:
    • Instead they run PREPARE on their query strings and output the hash of the query and the prepare result as randomly named JSON files to QUERY_PREPARE_OUT
    • Then emit panics instead of their normal expansion so compilation can still finish.
  • The subcommand then gathers the files inside QUERY_PREPARE_OUT and writes them to a single JSON file in a predetermined location in the project (TBD)
    • These separate scatter/gather steps is because the macro expansions may run in parallel, so we don't want them all to clobber the same file.
    • This new file is intended to be checked-in to source control where it can be used even without a database connection, e.g. in CI.
    • If two queries have the same hash, the subcommand deduplicates the entries by asserting that the PREPARE data is the same.
  • If the file exists when query!() is later expanded normally, the macro checks in the file for the hash of its query string and uses that data instead of connecting to the database.
    • If the hash isn't found it emits a compile error saying to run cargo sqlx prepare again.
    • We can also support cargo sqlx prepare --check which does the opposite; it reruns PREPARE for the cached query strings and asserts that the result is the same.

We can then later extend the cargo-sqlx command to add migrations. With migrations in place we have a stronger guarantee that the schema we compiled and cached the PREPARE data against is the same as the one we are running against.

Questions:

  • Is prepare the right name for the sub-subcommand?
  • What should the cached file be named and where should it go?
  • Should we support a sqlx.toml to change the location and possibly other behaviors?
  • Should the macro use an environment variable like QUERY_PREPARE_OUT or look at a cfg flag? I'm thinking the former might fail to trigger rebuilds when set if Cargo doesn't know to watch for it but the latter might trigger a rebuild of the whole dep graph.
    • We could also just touch src/ to trigger a rebuild, no?
  • Do we want to support checking the cached file at runtime to ensure the PREPARE data we saved still matches what we get back from the database server?
@abonander abonander added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Jan 15, 2020
@abonander abonander self-assigned this Jan 15, 2020
@abonander abonander added this to the 0.3 milestone Jan 15, 2020
@abonander abonander removed the question Further information is requested label Jan 15, 2020
@mehcode
Copy link
Member

mehcode commented Jan 16, 2020

Is prepare the right name for the sub-subcommand?

I initially thought cargo sqlx build but prepare sounds better. You're preparing to build. And this should be run before cargo build.

What should the cached file be named and where should it go?

We probably need to introduce a sqlx.toml file. But as a default ... I would suggest (without a lot of weight) something like sqlx.json or sqlx.lock in project root.

Should we support a sqlx.toml to change the location and possibly other behaviors?

Yes. query! is going to have stuff. Like for mysql should TINY(1) be treated as i8 or bool. That'd be a good place to decide that, among other stuff like migrations.

Should the macro use an environment variable like QUERY_PREPARE_OUT or look at a cfg flag? [...]

It seems without say a force-rebuild option in cargo we would need to use the env variable and touch lib.rs and main.rs (if exists). I'd recommend to prefix that env variable under SQLX_.

Do we want to support checking the cached file at runtime to ensure the PREPARE data we saved still matches what we get back from the database server?

I'd call that out-of-scope for the first iteration of this. But I would say we should do this at some point. Definitely with a way to disable it (via sqlx.toml probably).


More questions I thought of:

  • Should we (re-)generate this sqlx.json file during regular cargo build ?

  • Should we have an offline param in sqlx.toml that disables any network in normal build ?

@abonander
Copy link
Collaborator Author

To force recompilation, in our macro shims we can do

const _: Option<&str> = env!("SQLX_PREPARE_OUT");

which informs rustc of the dependency on that env var.

@D1plo1d
Copy link
Contributor

D1plo1d commented Feb 15, 2020

I'll add that this effects containerized cross compilation - currently sqlx doesn't work with cross and a local postgres instance because you would need to port forward the postgres instance (which afaik cross doesn't support).

Should we (re-)generate this sqlx.json file during regular cargo build ?

If we integrate (re-)generation with cargo build I'd ask that we include a environment variable so that I can disable it for cross compilation (SQLX_OFFLINE?).

Otherwise (re-)generation in cargo build sounds like a good default to me.

@arlyon
Copy link
Contributor

arlyon commented Mar 28, 2020

To further @D1plo1d, this would also 'fix' builds in tools such as bazel as well (or any other isolated build systems such as docker). Having the schema-derived-metadata checked into source control for a given database state seems absolutely necesarry. It would also be great to be able to take this generic metadata and convert it back into the schema definitions that generated it, making sqlx a potent solution for tracking database changes (and potentially migrations) and placing it in a nice spot between raw sql and full blown ORMs

To provide input on your question @mehcode about regenerating during cargo build automatically, I personally would like to make sure that the file was always up-to-date for the sake of reproducibility so that I can go back through old commits / tags and guarantee identical behaviour but I don't think automatically rebuilding it is the answer (as we want migration points to be deliberate). In the long term, I would like to see this project break the build time dependency on the database entirely and take an offline-first approach (ie, one that only uses sqlx.json) by default, and a workflow that looks like this:

  • download some sqlx-powered project
  • set up the database and create the schemas for it (potentially using some command in cargo sqlx) or migrate my current schema to it (also potentially using some command in cargo sqlx)
  • make some changes to the source
  • make some changes to the schema
  • get warnings (or compiler errors) from sqlx that the schema doesn't match
  • run cargo sqlx prepare
  • check in the new sqlx.json with my source

Then, in my docker containers (or in rust cross, or bazel, etc.), I just compile, and it uses the sqlx.json that we know matches the source code being built. Additionally, at run time, my binaries could compare the build-time sqlx.json against the database to make sure it's being deployed against the right version.

@mehcode
Copy link
Member

mehcode commented May 31, 2020

This should now be available in master.

See instructions for testing: #249 (comment)

@mehcode mehcode closed this as completed May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants