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

Merge of #3427 (by @mpyw) and #3614 (by @bonsairobo) #3765

Merged
merged 31 commits into from
Mar 10, 2025
Merged

Conversation

abonander
Copy link
Collaborator

@abonander abonander commented Mar 2, 2025

Merges #3427 (by @mpyw) and #3614 (by @bonsairobo).

These both touch a lot of the same code and I didn't want to continually bother both authors with requests to rebase.

mpyw and others added 22 commits August 12, 2024 13:10
SQLite implementation is currently WIP
I have included `AtomicUsize` in `WorkerSharedState`. Ideally, it is not desirable to execute `load` and `fetch_add` in two separate steps, but we decided to allow it here since there is only one thread writing. To prevent writing from other threads, the field itself was made private, and a getter method was provided with `pub(crate)`.
This patch completes the plumbing of an optional statement from these methods to
`TransactionManager::begin` without any validation of the provided statement.

There is a new `Error::InvalidSavePoint` which is triggered by any attempt to
call `Connection::begin_with` when we are already inside of a transaction.
This makes the new method a non-breaking change.
Move the wrapper directly into the test that uses it instead.
@abonander
Copy link
Collaborator Author

I forgot that the reason why I held off merging #3614 was because I wanted to merge #3427 and they touch a lot of the same code. This PR will subsume both of those.

…an/begin-with' into 3614-rebased

# Conflicts:
#	sqlx-core/src/connection.rs
#	sqlx-postgres/src/transaction.rs
#	sqlx-sqlite/src/connection/worker.rs
#	sqlx-sqlite/src/transaction.rs
# Conflicts:
#	sqlx-mysql/src/any.rs
#	sqlx-postgres/src/any.rs
#	sqlx-sqlite/src/connection/mod.rs
#	tests/sqlite/sqlite.rs
@abonander abonander changed the title Rebase of #3614 (by @bonsairobo) Merge of #3427 (by @mpyw) and #3614 (by @bonsairobo) Mar 3, 2025
@abonander
Copy link
Collaborator Author

I've decided that it would be best to limit the public interface of #3427 to just is_in_transaction. Transaction "depth" is purely an affectation of SQLx and doesn't really mean anything from the perspective of the database If there is value in knowing how many savepoints are active in the current transaction, it'd be better to provide that as an explicitly named getter.

@abonander abonander merged commit 393b731 into main Mar 10, 2025
81 checks passed
@abonander abonander deleted the 3614-rebased branch March 10, 2025 21:29
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.

3 participants