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

Track null arguments in order to provide the appropriate type when converting them. #3298

Merged

Conversation

etorreborre
Copy link
Contributor

This PR fixes an issue occurring when:

  • A value None : Option<T> is bound to a query, and that query executed.
  • The same query is later executed with a value Some(t: T) : Option<T>.

Since queries are cached by default, the type of value considered for the bound parameter is Option::<i32>::None.
This will raise a Postgres error on the second execution "incorrect binary data format in bind parameter <n>".

The fix consists in storing the value type of null values (using the result of the encode function to determine if the value is null), and then using a None value with the correct type (for example Option::<String>::None to persist a nullable TEXT value).

I am quite new to the sqlx library and there might be a better implementation but this one seems to be quite consistent with the implementation for the MySQL driver.

@etorreborre etorreborre force-pushed the etorreborre/any-convert-null-to branch from a8daae3 to 9a8a9e1 Compare June 20, 2024 12:52
sqlx-core/src/any/arguments.rs Outdated Show resolved Hide resolved
@etorreborre etorreborre force-pushed the etorreborre/any-convert-null-to branch 4 times, most recently from b52e033 to 31086a7 Compare June 21, 2024 10:53
@etorreborre
Copy link
Contributor Author

@abonander the failures seem to be related to some recent Postgres changes on main. Do you confirm this?

@etorreborre
Copy link
Contributor Author

Hi @abonander. Is the latest version I pushed ok?

@etorreborre etorreborre force-pushed the etorreborre/any-convert-null-to branch from f919e0b to 1154926 Compare July 9, 2024 10:09
@abonander abonander merged commit 93f3d79 into launchbadge:main Jul 11, 2024
64 checks passed
@etorreborre
Copy link
Contributor Author

Thanks @abonander!

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

Successfully merging this pull request may close these issues.

2 participants