Skip to content

Add NpgsqlTransactionOptions for read-only/deferrable transactions - #6629

Open
bjornharrtell wants to merge 2 commits into
npgsql:mainfrom
bjornharrtell:readonly-deferrable-transactions
Open

Add NpgsqlTransactionOptions for read-only/deferrable transactions#6629
bjornharrtell wants to merge 2 commits into
npgsql:mainfrom
bjornharrtell:readonly-deferrable-transactions

Conversation

@bjornharrtell

Copy link
Copy Markdown
Contributor

Adds NpgsqlConnection.BeginTransaction(Async) overloads accepting a new NpgsqlTransactionOptions flags enum (ReadOnly, Deferrable), allowing transactions to be started with these options without an extra roundtrip (e.g. SET TRANSACTION READ ONLY).

Ref #867

Adds NpgsqlConnection.BeginTransaction(Async) overloads accepting a new
NpgsqlTransactionOptions flags enum (ReadOnly, Deferrable), allowing
transactions to be started with these options without an extra
roundtrip (e.g. SET TRANSACTION READ ONLY).

Fixes npgsql#867
@bjornharrtell
bjornharrtell requested a review from roji as a code owner August 10, 2026 17:45
Copilot AI lite review requested due to automatic review settings August 10, 2026 17:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new NpgsqlTransactionOptions flags enum (e.g., ReadOnly, Deferrable) and adds BeginTransaction/BeginTransactionAsync overloads on NpgsqlConnection to start transactions with these options directly in the initial BEGIN statement (avoiding a follow-up SET TRANSACTION roundtrip).

Changes:

  • Add NpgsqlTransactionOptions enum to represent Npgsql-specific transaction start options.
  • Add new sync/async BeginTransaction overloads accepting NpgsqlTransactionOptions, and wire them into transaction initialization.
  • Add tests verifying READ ONLY and DEFERRABLE behavior (sync path).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/Npgsql.Tests/TransactionTests.cs Adds coverage for read-only and deferrable transaction options.
src/Npgsql/PublicAPI.Unshipped.txt Registers the newly added public overloads in the public API tracking file.
src/Npgsql/NpgsqlTransactionOptions.cs Introduces the new flags enum for transaction start options.
src/Npgsql/NpgsqlTransaction.cs Extends transaction initialization to emit a dynamic BEGIN statement when options are requested.
src/Npgsql/NpgsqlConnection.cs Adds new BeginTransaction/BeginTransactionAsync overloads that accept NpgsqlTransactionOptions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Npgsql/NpgsqlConnection.cs Outdated
Comment thread src/Npgsql/PublicAPI.Unshipped.txt Outdated
Comment thread src/Npgsql/NpgsqlTransaction.cs
Comment thread test/Npgsql.Tests/TransactionTests.cs
- Remove single-arg BeginTransaction(Options)/BeginTransactionAsync(Options, ...)
  overloads: combined with the existing IsolationLevel overloads, these made
  calls like BeginTransaction(default) ambiguous.
- Thread async/cancellationToken through NpgsqlTransaction.Init into WriteQuery
  instead of always blocking synchronously.
- Update PublicAPI.Unshipped.txt accordingly.
- Add async test coverage for the ReadOnly/Deferrable options.
Copilot AI review requested due to automatic review settings August 10, 2026 18:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Npgsql/NpgsqlTransaction.cs:136

  • NpgsqlTransactionOptions isn't validated before generating the BEGIN statement. Invalid flag values (unknown bits) will be silently ignored, and Deferrable without ReadOnly and Serializable will cause a server-side error at execution time. Consider validating flags/combinations and throwing an argument exception early with a clear message.
            var sb = new StringBuilder("BEGIN TRANSACTION ISOLATION LEVEL ").Append(isolationLevelText);
            if ((options & NpgsqlTransactionOptions.ReadOnly) != 0)
                sb.Append(" READ ONLY");
            if ((options & NpgsqlTransactionOptions.Deferrable) != 0)
                sb.Append(" DEFERRABLE");

@roji roji 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.

Thanks, looks good! See one small nit, other than that looks ready to merge.


// Unlike the isolation levels above, these options can be combined in many ways, making it impractical to pregenerate
// messages for all combinations; the BEGIN statement is written out and sent like a regular (prepended) query instead.
await _connector.WriteQuery(sb.ToString(), async, cancellationToken).ConfigureAwait(false);

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.

(from @vonzshik) you can just do a synchronous write here, assuming that there will always be enough space in the buffer; we already make that assumption above when we call PrependInternalMessage. At that point everything here is sync and you can also inline the local method.

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