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

Is it possible to decouple compile-time checked queries from the use of prepared statements? #368

Closed
bjeanes opened this issue Jun 2, 2020 · 8 comments

Comments

@bjeanes
Copy link
Contributor

bjeanes commented Jun 2, 2020

I know the answer is "no" currently, so I guess this is potentially a feature request.

Prepared statements in Postgres cannot be used in all cases. For instance, it is incompatible with pgbouncer (unless this has changed in the last year or so). In some deployments there is no option other than to connect to a pgbouncer sidecar for DB connectivity.

I wonder if there is potential to make the use of prepared statements at runtime a separate choice to having compile-time checked queries. Currently, the API has conflated these two decisions, to the best of my understanding.

@mehcode
Copy link
Member

mehcode commented Jun 2, 2020

Support for PgBouncer is being tracked separately but this is for sure a decision we can make separately. The runtime and compile-time layers are mostly separate.

From what I understand PgBouncer supports prepared statements but does not support caching of them. We could add a general configuration for 'prepared statement cache ttl' which a PgBouncer user could set to zero or something.

@bjeanes
Copy link
Contributor Author

bjeanes commented Jun 2, 2020

From what I understand PgBouncer supports prepared statements but does not support caching

Yeah that sounds familiar. I may be mistaking the fact that in other frameworks I had to disable them altogether (perhaps because they could not just disable caching). Either way, in evaluating this project, this came up to me as a potential gotcha.

There may be other scenarios in which prepared statements can't be used, but I am not specifically aware of them.

De-conflating these seems useful in the case where you do want prepared statements but can't, for some reason, pre-compile them. In a case I am contending with, I am include_str!()ing a large query, and the query! macro must take include_str! literally as a token instead of evaluating it and using the query it returns.

This example could possibly be resolved with improvements to query! but my point is more illustrative that there may be additional reasons why prepared statements and compile-time checked queries should be orthogonal concerns.

Thanks for your fast response!

@abonander
Copy link
Collaborator

In a case I am contending with, I am include_str!()ing a large query, and the query! macro must take include_str! literally as a token instead of evaluating it and using the query it returns.

We have query_file!() for this situation, although the path is unfortunately not relative to the source file's directory as there's no stable way we can determine that currently; instead it's relative to CARGO_MANIFEST_DIR (the directory containing Cargo.toml for the current crate).

We use prepared statements because it's the only way the databases give us any insight into the query as far as inputs and outputs go. Replacing that would involve writing our own SQL parser and analyzer, basically reimplementing a good chunk of the server-side frontend, for each database flavor we intend to support. We may end up doing that anyway for a number of reasons but that's still a significant undertaking.

For queries that cannot be prepared and are expected to have no inputs or outputs we could instead, potentially, execute them in a transaction that is never committed; however, MySQL doesn't support schema-changing operations in transactions which excludes all but a few edge cases. We also cannot infer when a query cannot be prepared without at least naively parsing the input SQL so this would have to be a different macro entirely anyway.

@bjeanes
Copy link
Contributor Author

bjeanes commented Jun 2, 2020

Yup, I expected that there would be some pretty gnarly stuff to consider here. Thanks for the additional context @abonander!

@bjeanes
Copy link
Contributor Author

bjeanes commented Jun 2, 2020

We have query_file!() for this situation, although the path is unfortunately not relative to the source file's directory as there's no stable way we can determine that currently; instead it's relative to CARGO_MANIFEST_DIR (the directory containing Cargo.toml for the current crate).

Good to know! I'll take advantage of that from now on :)

@bjeanes
Copy link
Contributor Author

bjeanes commented Jun 2, 2020

I think that in terms of behaviour for what originally prompted this issue, I think that it would be reasonable to require a DB connection at compile time which uses prepared statements, but the code that it outputs could be the raw query string executed in text mode but decoded to the types whose metadata was collected at compile time. Does that even make sense?

@abonander
Copy link
Collaborator

abonander commented Jun 2, 2020

The solution to #121 is going to be extensible to theoretically any kind of configuration options we want to pass into the macros; we could consider adding this as a flag to change the generated code. It doesn't feel right to change it in the general case, however.

I think instead I agree with @mehcode about a flag on the connection itself to avoid caching prepared statements; it'll require some experimentation as that may not be enough and the flag may have to turn off prepared statements altogether and instead submit the query string raw. sqlx::query() would function identically to the user and to the generated code, only the commands submitted would change (and describe() would have to just error I guess).

If you want to add bind parameters we would have to manually interpolate them into the query string, however solving #300 should basically take care of that.

@mehcode
Copy link
Member

mehcode commented Jun 10, 2020

Closing as this discussion seems resolved. Please re-open if not.

@mehcode mehcode closed this as completed Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants