Skip to content

Accept transaction isolation levels, but always use REPEATABLE READ - #3063

Merged
fulghum merged 3 commits into
mainfrom
fulghum/tx-isolation-syntax
Aug 11, 2026
Merged

Accept transaction isolation levels, but always use REPEATABLE READ#3063
fulghum merged 3 commits into
mainfrom
fulghum/tx-isolation-syntax

Conversation

@fulghum

@fulghum fulghum commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Follow Dolt's existing behavior to allow clients to specify a transaction isolation level, but still always use REPEATABLE READ.

Also includes an update to the Doltgres SQL parser, to allow us to differentiate between specified isolation levels in the future.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
Main PR
covering_index_scan_postgres 2215.70/s 2181.26/s -1.6%
groupby_scan_postgres 157.05/s 144.76/s -7.9%
index_join_postgres 717.66/s 710.93/s -1.0%
index_join_scan_postgres 933.93/s 919.61/s -1.6%
index_scan_postgres 33.62/s 33.16/s -1.4%
oltp_delete_insert_postgres 917.17/s 876.83/s -4.4%
oltp_insert 822.19/s 797.76/s -3.0%
oltp_point_select 3764.68/s 3767.56/s 0.0%
oltp_read_only 3675.40/s 3637.62/s -1.1%
oltp_read_write 2703.27/s 2573.59/s -4.8%
oltp_update_index 865.38/s 828.91/s -4.3%
oltp_update_non_index 941.65/s 892.01/s -5.3%
oltp_write_only 1993.01/s 1913.28/s -4.1%
select_random_points 2227.44/s 2231.38/s +0.1%
select_random_ranges 1682.02/s 1707.03/s +1.4%
table_scan_postgres 33.54/s 33.05/s -1.5%
types_delete_insert_postgres 917.00/s 917.73/s 0.0%
types_table_scan_postgres 14.68/s 14.51/s -1.2%

@itoqa

itoqa Bot commented Aug 10, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: c6544b4: 14 test cases ran, 1 failed ❌, 13 passed ✅.

Summary

Coverage spans transaction syntax and isolation modes, read-only/read-write behavior, commit and rollback data visibility, cross-session consistency, and invalid or repeated transaction requests. The normal transaction flows are healthy, while an edge-case retry path exposes unsafe state changes that can prevent an expected write from being saved.

Not safe to merge yet — an attributable high-severity failure allows a repeated transaction request to silently replace an active writable state with read-only behavior, causing writes to be rejected and user changes to be lost. The issue affects transaction integrity and requires correction before merging.

Tests run by Ito

View full run

Result Severity Type Description
High severity Wiring The second begin request returned successfully, but it changed the active transaction to read-only. The insert was rejected and the later commit left the table empty instead of preserving the original writable transaction and committing the row.
Combination The unsupported transaction command was rejected, and the session stayed out of a transaction. The later insert was handled as a normal standalone insert, and a new valid transaction also worked.
Combination The unsupported transaction command failed without opening a transaction or writing data. A valid retry then committed row 302, and a second session saw only that row.
Isolation The database accepted a transaction marked READ UNCOMMITTED and rolled it back without an error. The session kept its documented read committed engine behavior while accepting the requested syntax.
Isolation The server accepted a SNAPSHOT transaction, started it successfully, and rolled it back without an error.
Isolation Both transaction isolation requests succeeded and rolled back cleanly. The parser source assigns separate values to REPEATABLE READ and SERIALIZABLE, and formats each name in uppercase.
Isolation The generated command tests passed, and the live SQL session accepted all five isolation forms and rolled each transaction back cleanly.
Session The SQL session accepted the transaction commands, committed row 1, rolled back a later transaction, and kept row 1 visible at the end.
Session The generated BEGIN and START TRANSACTION checks accepted the supported isolation levels and read modes. Every exercised combination converted as expected.
Transaction The server accepted the requested transaction mode, saved row 1, and returned it after commit.
Transaction The server accepted SERIALIZABLE with READ WRITE, saved row 2, and then returned that row from a REPEATABLE READ transaction.
Transaction Two sessions using different isolation labels kept the same repeatable-read visibility rules. Neither session saw the other session's uncommitted row, and both committed rows remained available afterward.
Wiring The read-only transaction started successfully, the existing row was readable, and the attempted insert was rejected before rollback closed the transaction.
Wiring The transaction accepted the isolation and READ WRITE options, saved the inserted row, and returned it after commit.

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread server/ast/begin_transaction.go
@itoqa

itoqa Bot commented Aug 10, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Reportc6544b451f9d06: 13 test cases ran, 1 new failure ❌, 1 fixed ✅, 11 passing ✅.

Diff Summary

Coverage spans normal transaction commits and rollbacks, data visibility, isolation and read/write modes, session readiness, and nested or malformed transaction requests across ordinary and prepared execution paths. Most exercised behaviors remain healthy, but a transaction-state edge case compromises read-only enforcement.

Not safe to merge yet — a PR-attributable high-severity failure allows writes in a transaction expected to be read-only after rollback, undermining data-protection guarantees. The remaining exercised transaction behaviors pass, but this is a merge blocker.

Tests run by Ito

View full run

Result State Severity Type Description
❌ New Failure High severity Transaction The INSERT succeeded inside the fresh read-only transaction instead of returning a read-only transaction error.
❌->✅ Fixed Wiring Starting a second transaction did not replace the first one. The insert succeeded, one commit completed, and the saved row was returned afterward.
Passing Isolation The corrected transaction sequence accepted both isolation requests, kept the first transaction writable, and committed the inserted row. The earlier error came from missing commas in the test SQL, not from the application.
Passing Isolation The unsupported DEFERRABLE option was rejected, and the same transaction still accepted SELECT 1 and COMMIT.
Passing Isolation The unsupported duplicate transaction command returned an error without ending or replacing the active transaction. Both marker rows were then committed together and were visible from a new session.
Passing Session Both prepared BEGIN commands completed successfully, the original writable transaction stayed active, and the inserted row was committed and read back.
Passing Session The invalid duplicate transaction command returned an error. After rollback, the same connection started a new transaction and completed SELECT 1 normally.
Passing Session The simple and prepared query paths both kept the original writable transaction active after a duplicate BEGIN. Inserts committed successfully, and both sessions ended in the expected idle state.
Passing Session The connection kept the transaction status active during BEGIN, duplicate BEGIN, and rollback, accepted a new transaction afterward, and returned to idle only after COMMIT.
Passing Transaction The second BEGIN finished normally, and the same connection immediately ran SELECT 1 and returned one row before COMMIT succeeded.
Passing Transaction A malformed duplicate transaction command returned an error, while the original transaction stayed usable and committed row 601 successfully.
Passing Wiring The first read-only transaction stayed read-only after a nested writable begin. The insert was rejected, and the session remained usable until rollback.
Passing Wiring A read-only transaction request did not change the active writable transaction. The insert succeeded, one row was committed, and the row was still present after reconnecting.
⏸️ Skipped Combination The unsupported transaction command was rejected, and the session stayed out of a transaction. The later insert was handled as a normal standalone insert, and a new valid transaction also worked.
⏸️ Skipped Combination The unsupported transaction command failed without opening a transaction or writing data. A valid retry then committed row 302, and a second session saw only that row.
⏸️ Skipped Isolation The database accepted a transaction marked READ UNCOMMITTED and rolled it back without an error. The session kept its documented read committed engine behavior while accepting the requested syntax.
⏸️ Skipped Isolation The server accepted a SNAPSHOT transaction, started it successfully, and rolled it back without an error.
⏸️ Skipped Isolation Both transaction isolation requests succeeded and rolled back cleanly. The parser source assigns separate values to REPEATABLE READ and SERIALIZABLE, and formats each name in uppercase.
⏸️ Skipped Isolation The generated command tests passed, and the live SQL session accepted all five isolation forms and rolled each transaction back cleanly.
⏸️ Skipped Session The SQL session accepted the transaction commands, committed row 1, rolled back a later transaction, and kept row 1 visible at the end.
⏸️ Skipped Session The generated BEGIN and START TRANSACTION checks accepted the supported isolation levels and read modes. Every exercised combination converted as expected.
⏸️ Skipped Transaction The server accepted the requested transaction mode, saved row 1, and returned it after commit.
⏸️ Skipped Transaction The server accepted SERIALIZABLE with READ WRITE, saved row 2, and then returned that row from a REPEATABLE READ transaction.
⏸️ Skipped Transaction Two sessions using different isolation labels kept the same repeatable-read visibility rules. Neither session saw the other session's uncommitted row, and both committed rows remained available afterward.
⏸️ Skipped Wiring The read-only transaction started successfully, the existing row was readable, and the attempted insert was rejected before rollback closed the transaction.
⏸️ Skipped Wiring The transaction accepted the isolation and READ WRITE options, saved the inserted row, and returned it after commit.

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread server/connection_handler.go

@zachmu zachmu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fulghum
fulghum enabled auto-merge August 10, 2026 23:54
@itoqa

itoqa Bot commented Aug 11, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report51f9d0649d5cc9: 9 test cases ran, 1 fixed ✅, 7 passing ✅, 1 additional finding ⚠️.

Diff Summary

The run covers core transaction behavior across read-only and read-write flows, rollback and savepoint recovery, nested and multi-statement requests, simple and prepared interactions, commit visibility, and rejection of unsupported options. It exercises both normal business flows and edge cases around session state and transaction isolation, with overall behavior matching expectations except for a separate isolation-semantics observation.

Safe to merge — the only failure is an unrelated, medium-severity isolation behavior finding explicitly not attributable to this PR, with no regressions or PR-owned failures identified. It is a flag for later rather than a merge blocker.

Tests run by Ito

View full run

Result State Severity Type Description
❌->✅ Fixed Transaction After a rollback, the next read-only transaction was enforced correctly. The attempted insert was rejected and the table stayed empty.
Passing Isolation After a rollback, unsupported transaction options still return errors and do not start a transaction. Both DEFERRABLE and AS OF were rejected as expected.
Passing Session A prepared insert after rollback was rejected because the new transaction was read-only, as expected.
Passing Session The simple and prepared query paths both rejected the write in the new read-only transaction, then accepted a later rollback.
Passing Session Rollback returned the expected command results, the read-only insert failed and the connection recovered, and a later committed write was visible from a new session.
Passing Transaction Rolling back to a savepoint did not end the writable transaction. The insert succeeded, the transaction committed, and the saved row was returned by the final query.
Passing Transaction The rollback ended the old transaction, the next read-only transaction blocked the insert, and a later read-write transaction committed row 9.
Passing Wiring After ending a read-only transaction, the next read-write transaction accepted an insert, committed it, and returned the saved row.
⏸️ Skipped Isolation The corrected transaction sequence accepted both isolation requests, kept the first transaction writable, and committed the inserted row. The earlier error came from missing commas in the test SQL, not from the application.
⏸️ Skipped Isolation The unsupported DEFERRABLE option was rejected, and the same transaction still accepted SELECT 1 and COMMIT.
⏸️ Skipped Isolation The unsupported duplicate transaction command returned an error without ending or replacing the active transaction. Both marker rows were then committed together and were visible from a new session.
⏸️ Skipped Session Both prepared BEGIN commands completed successfully, the original writable transaction stayed active, and the inserted row was committed and read back.
⏸️ Skipped Session The invalid duplicate transaction command returned an error. After rollback, the same connection started a new transaction and completed SELECT 1 normally.
⏸️ Skipped Session The simple and prepared query paths both kept the original writable transaction active after a duplicate BEGIN. Inserts committed successfully, and both sessions ended in the expected idle state.
⏸️ Skipped Session The connection kept the transaction status active during BEGIN, duplicate BEGIN, and rollback, accepted a new transaction afterward, and returned to idle only after COMMIT.
⏸️ Skipped Transaction The second BEGIN finished normally, and the same connection immediately ran SELECT 1 and returned one row before COMMIT succeeded.
⏸️ Skipped Transaction A malformed duplicate transaction command returned an error, while the original transaction stayed usable and committed row 601 successfully.
⏸️ Skipped Wiring Starting a second transaction did not replace the first one. The insert succeeded, one commit completed, and the saved row was returned afterward.
⏸️ Skipped Wiring The first read-only transaction stayed read-only after a nested writable begin. The insert was rejected, and the session remained usable until rollback.
⏸️ Skipped Wiring A read-only transaction request did not change the active writable transaction. The insert succeeded, one row was committed, and the row was still present after reconnecting.
⚠️ Additional Finding Medium severity Isolation All five corrected BEGIN ISOLATION LEVEL sequences completed, but SHOW transaction_isolation returned read committed every time. The accepted isolation setting did not result in the required repeatable-read behavior.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Isolation setting is ignored after rollback
  • Severity: Medium Medium severity
  • Description: All five corrected BEGIN ISOLATION LEVEL sequences completed, but SHOW transaction_isolation returned read committed every time. The accepted isolation setting did not result in the required repeatable-read behavior.
  • Impact: Transactions that request the documented isolation behavior run with weaker read consistency instead. Applications that rely on repeatable reads may see changing results during a transaction, though there is no evidence of data loss or corruption.
  • Steps to Reproduce:
    1. Open one local PostgreSQL-compatible session.
    2. For each of READ UNCOMMITTED, READ COMMITTED, REPEATABLE READ, SERIALIZABLE, and SNAPSHOT, run BEGIN READ WRITE; ROLLBACK; BEGIN ISOLATION LEVEL ; SELECT 1; SHOW transaction_isolation; COMMIT.
    3. Check the value returned by SHOW transaction_isolation for each new transaction.
    4. Compare the returned value with the documented compatibility behavior, which requires repeatable-read execution for every accepted spelling.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The parser correctly preserves the requested isolation value: postgres/parser/sem/tree/txn.go:35-65 defines five isolation enum values and maps READ UNCOMMITTED, READ COMMITTED, REPEATABLE READ, SERIALIZABLE, and SNAPSHOT to them. However, server/ast/begin_transaction.go:31-34 states that accepted isolation syntax must use REPEATABLE READ semantics, while nodeBeginTransaction at lines 45-58 only converts node.Modes.ReadWriteMode into a Vitess BEGIN characteristic. It never reads node.Modes.Isolation and never emits or applies a repeatable-read setting. The resulting transaction therefore uses the session default from server/config/parameters_list.go:3756-3765, where transaction_isolation defaults and resets to read committed. The smallest practical fix is to make the BEGIN conversion explicitly establish the documented repeatable-read behavior, or to set the corresponding transaction isolation parameter when building the engine transaction, while continuing to ignore the individual compatibility spellings.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@github-actions

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 18941 18948
Failures 23149 23142
Partial Successes1 5340 5338
Main PR
Successful 45.0012% 45.0178%
Failures 54.9988% 54.9822%

${\color{red}Regressions (1)}$

copyselect

QUERY:          drop table test3;
RECEIVED ERROR: Doltgres sent additional messages after ReadyForQuery

${\color{lightgreen}Progressions (10)}$

insert_conflict

QUERY: begin transaction isolation level read committed;
QUERY: begin transaction isolation level repeatable read;
QUERY: begin transaction isolation level serializable;
QUERY: begin transaction isolation level read committed;
QUERY: begin transaction isolation level repeatable read;
QUERY: begin transaction isolation level serializable;

tidscan

QUERY: BEGIN ISOLATION LEVEL SERIALIZABLE;

transactions

QUERY: START TRANSACTION ISOLATION LEVEL REPEATABLE READ; INSERT INTO trans_abc VALUES (15); COMMIT AND CHAIN;
QUERY: START TRANSACTION ISOLATION LEVEL REPEATABLE READ; INSERT INTO trans_abc VALUES (16); ROLLBACK AND CHAIN;

vacuum

QUERY: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

@fulghum
fulghum merged commit 7b8cb0f into main Aug 11, 2026
25 checks passed
@fulghum
fulghum deleted the fulghum/tx-isolation-syntax branch August 11, 2026 01:13
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