- Notifications
You must be signed in to change notification settings - Fork514
Work with pools that don't support prepared statements#1147
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Introduce a new `query_with_param_types` method that allows to specify Postgres type parameters. This obviated the need to use prepared statementsjust to obtain parameter types for a query. It then combines parse, bind, and execute in a single packet.Related:rust-postgres#1017,rust-postgres#1067
| ) ->implExactSizeIterator<Item =(&'adynToSql,Type)> +'a{ | ||
| s.iter() | ||
| .map(|(param, param_type)|(*paramas_, param_type.clone())) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There's no need to factor this into a separate function here since it's only being called one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed. Earlier thought was to allow access to the rawRowStream.
tokio-postgres/src/query.rs Outdated
| }) | ||
| } | ||
| enumQueryProcessingState{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It feels like this is a lot of boilerplate over a fewmatch responses.next().await? { ... } in a sequence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed.
Thinking about it, it seems like we could make query_typed do this when invoked with a non-prepared statement instead of adding a new method. |
Did you mean |
Ah yeah my idea didn't really make much sense :D |
Let's rename the method to |
Done! |
LGTM but there's one small clippy error to fix. |
Fixed. |
Thanks! |
Thank you! |
Withrust-postgres/rust-postgres#1165 merged and a new release with that change available in 0.7.12, we no longer need to use our fork.Compared torust-postgres/rust-postgres#1147, there was a name change for `query_typed`, so this commit takes care of that.
Currently, each query requires three RTTs to the DB. By switching to typed queries, wecan do the parse, bind and execute all in a single RTT.Queries which use `SqlNamespaceStatus` were not ported due to me not being able to figureout which type they have. It's a user-generated type, and I'm not sure there's a matchingtype in `postgres-types::Type`. In any case, this just means we eat the extra RTTs whencreating or updating namespaces. This feels fine to me, since at least the hot path isable to be typed.Related tokio-postgres PR:rust-postgres/rust-postgres#1147
Introduce a new
query_with_param_typesmethod that allows to specify Postgres type parameters. This obviated the need to use prepared statements just to obtain parameter types for a query. It then combines parse, bind, and execute in a single packet.Related:#1017,#1067