- Notifications
You must be signed in to change notification settings - Fork516
Add parameter and column formatting to query_raw()#747
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
postgres-types/src/lib.rs Outdated
| ) ->Result<IsNull,Box<dynError +Sync +Send>>; | ||
| /// Specifies the message format of the value | ||
| fnformat(&self) ->Format{ |
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.
I don't know how I feel about exposing this on ToSql, since the same can't really be done for FromSql.
An alternative I've been thinking about is to have a separate query method that operates entirely in "string mode", both for parameters and the resulting rows.
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.
That's a really good point about the lack of equivalent handle on the returned types. It would be nice to let both parameters and returned types have independently-configurable formats like Postgres itself, but a text-only query variant (query_text, perhaps?) would probably cover those cases well-enough.
If we wanted to keep the configurability of each input and output's format, what about adding that configuration toStatement instead? Or tracking the configuration of those formats in aStatement, generated throughprepare_typed or some otherprepare_* method?
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.
An alternative I've been thinking about is to have a separate query method that operates entirely in "string mode", both for parameters and the resulting rows.
@sfackler This would be wonderful; is it anything you've thought more about? Passing arbitrary types as strings is particularly useful, since I could pass any number as a string without knowing the exact intended type and Postgres itself can figure it out. Being able to then load those numbers as strings would also be wonderful. My use case might not be typical, though, so I might be the only person wanting this.
@sfackler as a possible implementation of a "string mode" query method that lets users specify their own formats for inputs and outputs, I've added I decided against a separate method since Also not sure which check failed on this latest commit, since I don't have access to CircleCI, but let me know what it is and I'll fix it. |
|
In addition to the fact that it breaks backwards compatibility, I don't think that query_raw is the right thing to modify, since it still works with ToSql/FromSql which are defined to work with binary formats. |
gah, sorry, forgot about the sync client. Updating! |
If one wants to mix-and-match both input and output formats (which I would like to), I don't think this requirement is avoidable. Nor is it a bad thing, I don't think... we can leverage |
d156c2e to32b06eeCompareClosing in favor of#836 |
Uh oh!
There was an error while loading.Please reload this page.
Summary
This PR wouldclose#746
by adding aby addingformat(&self)method toToSql, defaulting toFormat::Binaryfor backwards compatibility and ease-of-useparam_formatsandcolumn_formatsto thequery_raw()method.Notes
This implementation allocatestwo new vectors, which might not be acceptable. Running benchmarks showed no consistent real-world performance differences on my machine, but YMMV. It would be possible to avoid those allocations ifencode_bindcould take anIntoIterator<Item = &P>instead ofP, I think, but that would certainly be a lot more work to get changed.