Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The test needs a PostgreSQL 19+ version guard before schema creation because its standard_conforming_strings=off configuration prevents connections from opening, so neither case exercises type loading.
1 open issue
What changed in this PR
Fixes #6732 by always quoting savepoint names, allowing reserved words such as user and select. The PR also includes the pending #6707 changes that centralize quoting and correct replication command formatting.
Changes:
- Always quote names in savepoint creation, rollback, and release.
- Add shared quoting helpers for replication commands and SQL, plus validation for required pgoutput option values.
- Add regression tests for reserved words, replication names and options, and schema names containing backslashes.
| File | Description |
|---|---|
| test/Npgsql.Tests/TransactionTests.cs | Tests reserved-word savepoints and updates expected SQL. |
| test/Npgsql.Tests/Replication/ReplicationQuotingTests.cs | Tests replication names, options, and publication quoting. |
| test/Npgsql.Tests/QuotingTests.cs | Tests shared quoting helpers. |
| test/Npgsql.Tests/ConnectionTests.cs | Tests schema quoting with backslash escapes enabled. |
| src/Npgsql/VolatileResourceManager.cs | Quotes two-phase transaction identifiers as literals. |
| src/Npgsql/Util/Statics.cs | Adds shared identifier and literal quoting helpers. |
| src/Npgsql/Replication/ReplicationConnection.cs | Quotes slot names in read and drop commands. |
| src/Npgsql/Replication/PhysicalReplicationConnection.cs | Quotes physical replication slot names and rejects null names. |
| src/Npgsql/Replication/PgOutput/PgOutputReplicationOptions.cs | Escapes publication names in option lists. |
| src/Npgsql/Replication/Internal/LogicalReplicationConnectionExtensions.cs | Quotes logical replication inputs and validates required options. |
| src/Npgsql/PostgresDatabaseInfo.cs | Quotes schema literals safely for type loading. |
| src/Npgsql/NpgsqlTransaction.cs | Always quotes savepoint names and removes conditional quoting. |
| src/Npgsql/NpgsqlCommand.cs | Uses the shared helper for named procedure arguments. |
Review effort: Balanced
Brar
force-pushed
the
tx/savepoint-names
branch
from
November 1, 2026 11:18
d5b5204 to
f01591d
Compare
NpgsqlTransaction quoted a savepoint name only when it didn't look like a lowercase identifier, so a name such as user or select went unquoted, and Save, Rollback and Release failed with 42601. Savepoint names are now always quoted, which keeps the meaning of every other name. The SQL changes with it, e.g. SAVEPOINT "foo" instead of SAVEPOINT foo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brar
force-pushed
the
tx/savepoint-names
branch
from
November 1, 2026 13:38
f01591d to
984200e
Compare
This branch has not been deployed
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.
NpgsqlTransactionquoted a savepoint name only when it didn't look like a lowercase identifier, so a reserved word such asuserorselectwent unquoted, andSave,RollbackandReleasefailed with 42601. Savepoint names are now always quoted, which keeps the meaning of every other name: a lowercase name means the same quoted or not.RequiresQuotingis gone.The SQL changes with it:
SAVEPOINT "foo"instead ofSAVEPOINT foo, and the same forROLLBACK TO SAVEPOINTandRELEASE SAVEPOINT, which shows in logs and traces. That's why this is a PR of its own rather than part of #6707, which otherwise sends the same SQL as before; whether it goes to the 10.0 hotfix branch is open.Tests
TransactionTests.Savepoint_named_like_a_reserved_word(user,select): without this change both fail with 42601 at the SAVEPOINT; with it they pass.Savepoint_prepends(mock) now expectsSAVEPOINT "foo".TransactionTestspass against PostgreSQL 18.6, in Debug and Release.Dependencies and merge order
Fixes #6732