feat: keep a session's transaction alive across batches - #124
Merged
Merged
Conversation
execute_query_batch already runs its statements on one pooled connection,
so BEGIN ... COMMIT inside a script works. The workflow the transaction
exists for did not: run BEGIN, inspect, run the changes, verify, then
COMMIT, each as its own run. Between runs the connection went back to the
pool, so the next run could land on a different one - the verify step saw
pre-transaction data and COMMIT reported no transaction in progress while
the real one stayed open elsewhere.
The pool recycles with RecyclingMethod::Fast, which resets nothing, so
that stranded connection also went back still inside the transaction,
holding its locks, and the next borrower ran inside it.
- transaction_effect() classifies a statement's effect on the surrounding
transaction from its leading keywords only, so a BEGIN in a string
literal or a PL/pgSQL block body is not mistaken for transaction
control, and ROLLBACK TO SAVEPOINT correctly leaves it open.
- session: registry of connections pinned per host session, a ROLLBACK
before any pinned connection returns to the pool, and an idle sweep so
an abandoned session cannot hold locks indefinitely.
- execute_query_batch honours session_id and answers with
{ results, in_transaction } when one is given, keeping the bare array
for hosts that do not send one.
- release_session RPC method; shutdown releases every pinned session.
- search_path is not re-applied to a reused connection: it would run
inside the open transaction and change what the rest of it sees.
Running statements one at a time is exactly how a transaction is driven -
BEGIN, the changes, a verifying SELECT, COMMIT - and the host sends each
of those through execute_query, not execute_query_batch. Without session
handling there, every run took a fresh pooled connection: the UPDATE
auto-committed on its own connection and the later COMMIT/ROLLBACK hit a
connection with no transaction, where PostgreSQL only warns and reports
success.
- Extract run_batch_in_session() from execute_query_batch so a single
statement and a batch take the same session path.
- execute_query honours session_id and answers with
{ result, in_transaction } when one is given, keeping the bare
QueryResult for hosts that do not send one.
PostgreSQL rolls the transaction back when COMMIT fails on its own (e.g. a deferred constraint), so keeping the session pinned left a connection held with no transaction behind it. Also classify ABORT, PREPARE TRANSACTION, COMMIT/ROLLBACK PREPARED and ... AND CHAIN, the last of which previously returned a connection to the pool with a fresh transaction still open.
The idle check only ran inside take(), so an abandoned session kept its transaction's locks until some other session happened to run a query. Hang sweep_idle() off the existing 10-minute pool cleanup timer instead.
Two overlapping calls for one session both found nothing pinned, each took a fresh connection, and the later store() rolled back the other's transaction. Each session now has its own lock held for the whole take, run, pin sequence, so an overlapping call waits its turn on the same connection, and release_session waits for a run still in flight.
It opens a new transaction whatever the prior state was.
Also covers the sweep forgetting a slot nothing uses.
…CTION ROLLBACK WORK/TRANSACTION TO SAVEPOINT was classified as closing, returning a connection to the pool mid-transaction, and only leading comments were skipped, so COMMIT -- and chain read as chaining and START /* x */ TRANSACTION was missed. transaction_effect now reads keywords with every comment skipped and drops the optional noise word.
release_all drained every slot before skipping busy ones, so a run still in flight pinned its connection into a slot nothing tracked, and it went back to the pool without a ROLLBACK. Busy slots now stay in the map, and a slot dropped while holding a connection closes it instead of returning it.
A failing statement came back as an RPC error, so the host never learned that a failed COMMIT had ended the transaction. With a session_id it is now a reply carrying the error and in_transaction.
…on-session # Conflicts: # CHANGELOG.md # tests/live_db.rs
Version suggestionBased on this PR's title (
This is informational only — no tag or release is created automatically yet. |
aesslinger
added a commit
that referenced
this pull request
Sep 29, 2026
Ships the four pending PRs merged since 1.0.0-rc.4: get_table_ddl for dump-schema-structure (#119), get_schema_snapshot + the batch metadata RPCs for one-round-trip ER diagrams (#123, #121), session/transaction pinning across execute_query/execute_query_batch runs (#124), and the uuid 1.26.0 -> 1.26.1 patch bump (#108). Also documents the .tabularium id/name split that landed since rc.4 (#117); the CI workflow split (#116) is intentionally omitted per the rc.3/rc.4 convention of not changelogging CI-only changes. Verified: cargo build --release; 356 unit + 30 live-DB against the local pg-tabularis-test podman container; the cross-repo 83-test byte-for-byte parity suite (POSTGRES_PLUGIN_BIN against tabularis' src-tauri/tests/postgres_integration parity*) passes 83/83; cargo clippy --all-targets -D warnings; cargo fmt --all --check; npx markdownlint CHANGELOG.md -- all clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Replaces #114 (a fork-based PR whose head branch couldn't be updated from the upstream side). All of @egertaia's original work is preserved here with full attribution; this branch simply merges it onto current
main(with #119/#123) and resolves the resulting conflicts.What
Companion to TabularisDB/tabularis#801, which plumbs the session id through the host. Either can merge first: the wire change is backward compatible in both directions.
execute_query_batchacquires one pooled connection for the whole batch, soBEGIN … COMMITinside a single script works. The workflow the transaction exists for does not:Each run is its own RPC call (and a one-statement run arrives as
execute_query, notexecute_query_batch), so the connection goes back to the pool in between. Run 2 can land on a different connection and show pre-transaction data; run 3 reportsthere is no transaction in progresswhile the real transaction stays open elsewhere.It is also a hazard for unrelated queries. The pool is configured with
RecyclingMethod::Fast, which resets nothing, so the stranded connection returns to the pool still inside the transaction, holding its locks, and the next borrower runs inside it.Fix
When the host sends a
session_id, a batch that leaves an explicit transaction open keeps its connection pinned to that session, so the next batch from the same session continues the same transaction. Committing, rolling back, an explicitrelease_session, shutdown, or 30 minutes idle releases it. A pinned connection is always rolled back before it goes back to the pool, so an open transaction can no longer leak into an unrelated query. Pinning is lazy: a session that never opens a transaction holds nothing and behaves exactly as before.transaction_effect()inhandlers/query.rs(BEGIN,START TRANSACTION,COMMIT,END,ROLLBACK,ABORT,PREPARE TRANSACTION, and... AND CHAIN), next to the existingstrip_leading_sql_commentsit builds on.session.rs: the registry, theROLLBACK-before-release rule, andsweep_idle(), run from the existing 10-minuterun_pool_cleanuptimer.run_batch_in_session, extracted fromexecute_query_batch, reuses the session's pinned connection and re-pins it if a transaction is still open afterwards.execute_querygoes through it as a one-statement batch, so a single statement and a batch take the same session path.release_sessionRPC method;shutdownreleases every pinned session.Wire compatibility (backward compatible, no companion required to merge)
session_idis optional on bothexecute_queryandexecute_query_batch. Without it the behaviour and the responses are byte-for-byte what they are today: a bareQueryResultand a bare array of per-statement results. Hosts older than tabularis#801 are unaffected — verified againstupstream/mainof tabularis, which sends nosession_idtoday.{ "result": {...}, "in_transaction": bool }and{ "results": [...], "in_transaction": bool }. The host side accepts both shapes permanently, and recognises the wrapper byin_transactionbeing present.release_sessionis a new method; a host that never calls it loses nothing (the idle sweep andshutdownstill reclaim connections).Status note
The transaction-pinning feature is dormant until tabularis#801 ships in a host release — no current host sends
session_id, so the new session path is unreachable in production until then. This PR is safe to merge independently (zero breakage, verified) and lights up when #801 lands. See the discussion on the closed #114 for the compatibility analysis.Merge onto current main
Merged
origin/main(with #119/#123) and resolved:CHANGELOG.md— took the bracketed## [Unreleased]convention; merged all Added/Fixed entries (theget_table_ddl/get_schema_snapshot/batch entries from feat: implement get_table_ddl RPC method for dump support #119/feat: implement get_schema_snapshot and batch metadata RPCs (#121) #123 plus this PR'ssession_id/release_sessionAdded and the rollback-before-release Fixed).tests/live_db.rs— kept all live tests (this PR's deferred-constraint session test + feat: implement get_table_ddl RPC method for dump support #119'sget_table_ddltests + feat: implement get_schema_snapshot and batch metadata RPCs (#121) #123'sget_schema_snapshottest).src/rpc.rs,handlers/query.rs,session.rs, andREADME.mdauto-merged cleanly (the dispatch additions occupy disjoint regions). All@egertaiacommits preserved with original attribution.Verification
cargo build --releasePOSTGRES_PLUGIN_BINagainsttabularis'src-tauri/tests/postgres_integration parity*): 83/83 pass — no existing behavior regressed.transaction_effect/in_transaction_aftercases).postgres:16instance — includingcommit_that_fails_on_a_deferred_constraint_releases_the_session, which had not previously been run live and now passes.cargo clippy --all-targets -- -D warnings;cargo fmt --all --check;npx markdownlint CHANGELOG.md— all clean.Supersedes and closes #114.
Generated with Lilly Code