fix(sqlite): query macro argument lifetime use inconsistent with other db platforms #3957
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes an inconsistency with allowed usage of references as arguments to query macros in the context of the sqlite feature compared to features for other db platforms, such as Postgres. The inconsistency is demonstrated in the first commit - this commit adds a test which would fail to compile without the remaining commits in the pr.
Compare the test added with this pr vs the existing test by the same name for Postgres:
The existing test for Postgres compiles and passes because
PgArguments
does not have a lifetime parameter. The new test for sqlite would fail to compile becauseSqliteArguments
had a lifetime parameter which causes the return value of thequery!
macro to be borrowing a reference to a value that is an argument toquery!
but goes out of scope whenquery!
returns.Does your PR solve an issue?
Yes - cannot pass a reference as a bind argument to
query!
unless the reference is established before thequery!
macro call (e.g. by assigning it to a variable withlet
).Is this a breaking change?
Yes, due to:
SqliteArguments
andSqliteArgumentValue
into_static()
on both of those typesSqliteArgumentsBuffer
type to wrapVec<SqliteArgumentValue>
SqliteArgumentValue::Text
to store anArc<String>
instead ofCow<str>
, and adding::TextSlice
for theArc<str>
variantCommits
The last commit (changing
AnyArguments::convert_to
toconvert_into
) could be moved out of this pr and into a new one focused on removing the lifetime arguments onAnyArguments
andAnyArgumentValue
to add similar support for references in the bind parameters when using theAny
driver.