Escape single quotes when building LIST partition bound literals

[jeremydmiller]

Partition bound values are interpolated into single-quoted SQL literals without escaping. Partition suffixes are fine — ListPartition.SanitizeSuffix character-whitelists them to [a-z0-9_] — but the values are not, and they are emitted straight into DDL.

Sites

1. FormatSqlValue does not double an embedded ' — src/Weasel.Postgresql/Tables/Partitioning/PartitionExtensions.cs:52

return $"'{value.ToString()}'";

2. FormatSqlValue returns already-quoted strings completely verbatim — same file, :50

if (value is string v && v.StartsWith("'") && v.EndsWith("'")) return v;

A value that merely starts and ends with ' gets no quoting or escaping at all. If some caller genuinely needs to pass a pre-formatted literal, that should be an explicit, separately named path rather than a shape heuristic applied to arbitrary input.

3. Same pattern in ManagedListPartitions.Partitions() — src/Weasel.Postgresql/Tables/Partitioning/ManagedListPartitions.cs:53

yield return new ListPartition(path.Key, path.Select(x => $"'{x.Key}'").ToArray());

Note this also feeds branch 2, since these values now start and end with '.

Where the values reach DDL

• ListPartition.cs:52-57 — CREATE TABLE … partition of … for values in ({Values.Join(", ")});
• ManagedListPartitions.cs:485 — alter table … attach partition … for values in ({values.Join(", ")});

Reached from AddPartitionToAllTables → additivelyMigrateTablesForNewPartitions (:349) → resolveBuckets (:397) → createOrWidenPartitionAsync (:357), and from ListPartitioning.AddPartition<T> (ListPartitioning.cs:43). The drop path reaches it too, via DropPartitionFromAllTablesForValue → rebindPartitionAsync (:142, :485).

Consumers use tenant ids as partition values, so any tenant id containing an apostrophe currently produces invalid DDL.

Also worth tightening

PostgresqlMigrator.AssertValidIdentifier (src/Weasel.Postgresql/PostgresqlMigrator.cs:337-355) rejects only null/whitespace, an embedded space, and over-length. It permits " and ;. Given it is the only identifier check in the stack, it should reject those too.

Related: there is no identifier validation in DbObjectName (src/Weasel.Core/DbObjectName.cs:9-20) or PostgresqlObjectName (src/Weasel.Postgresql/PostgresqlObjectName.cs:13-55) — neither contains a single throw. PostgresqlObjectName.From(...) is sometimes assumed to be a sanitizing boundary and is not; worth a doc comment saying so.

Quoting helpers that quote without escaping — lower priority, mostly configuration-derived input, but the same class:

File:line	Code
SchemaUtils.cs:79	return $"\"{name}\""; (and only quotes at all when the name is a keyword or has an uppercase char)
PostgresqlProvider.cs:378-384	returns the name verbatim when it already begins and ends with "
Tables/IndexDefinition.cs:348,357	$"\"{x}\"", COLLATE \"{Collation}\"
Migrations/DatabaseSpecification.cs:49	CREATE DATABASE \"{databaseName}\"
Functions/FunctionBody.cs:22	OWNER TO \"{owner}\";

Checked and clean

ManagedListPartitions' registry table DML binds every value as a real parameter (:78-81, :302-306, :499-503, :136-137, :194-197, :534-536, :221-224, :446-451). Only the table identifier is interpolated, from the constructor arg.

Tasks

• FormatSqlValue doubles embedded '
• Remove or explicitly gate the pre-quoted passthrough branch at :50
• ManagedListPartitions.Partitions() uses the escaping helper
• AssertValidIdentifier rejects " and ;
• Note on DbObjectName/PostgresqlObjectName that they do not validate
• Round-trip tests: a partition value containing ' creates, attaches, detaches and drops correctly

Not established: whether the SQL Server / Oracle / MySql / Sqlite Migrator subclasses have the equivalent AssertValidIdentifier gap (SqlServerMigrator.cs:140, OracleMigrator.cs:119, MySqlMigrator.cs:110, SqliteMigrator.cs:87).

[jeremydmiller]

Fixed in 9.21.1 (commit 26421ba, published to NuGet).

• FormatSqlValue doubles an embedded single quote, and the pre-quoted passthrough branch is gone. It now always formats a raw value and is explicitly documented as not idempotent.
• Callers legitimately holding formatted literals use new internal AddPartitionWithSqlLiterals / AddRangeWithSqlLiterals. Production read-back never depended on the removed branch — ListPartition.Parse passes catalog literals straight to the constructor — only the three delta tests simulating read-back did.
• ManagedListPartitions.Partitions() no longer hand-rolls its quotes.
• Weasel.SqlServer RangePartitioning.FormatSqlValue's catch-all arm got the same treatment.

Round-trip regression tests confirmed failing against 9.21.0 (a quote-bearing partition value executes its payload on both the create and the drop/rebind path) and passing after. Full Weasel.Postgresql suite green: 825 passed, 3 pre-existing skips.

Left open on this issue: PostgresqlMigrator.AssertValidIdentifier still permits " and ; — it rejects only null/whitespace, an embedded space, and over-length. Worth tightening, but it sits on a shared path where quote-wrapped names appear to circulate (see PostgresqlProvider.ToQualifiedName, which returns such names verbatim), so it deserves its own change and regression pass rather than riding along here. Also still open: the doc note that DbObjectName/PostgresqlObjectName perform no validation, and the lower-priority quoting helpers in the table at the top.

[jeremydmiller]

The remaining items are fixed in 9.23.0 (PR #419), completing the issue.

• AssertValidIdentifier rejects " and ; — the two characters that let an object name escape the statement it is written into. It also now rejects all whitespace rather than just the literal space it used to check, since a newline could otherwise introduce a -- comment into an unquoted name.
• PostgresqlIdentifierInvalidException gained a constructor that names the rule that was broken, so the message says which character was the problem. The existing single-argument constructor is untouched, and the message stopped blaming Marten for a decision that is Weasel's.
• DbObjectName and PostgresqlObjectName now carry the doc note that they do not validate — From quotes for rendering, which is not the same as checking that a name is safe.

The regression pass this was deferred for: full Weasel.Postgresql suite green (846 passed, 3 pre-existing skips), Weasel.Core green (50), and all CI providers green. Nothing on the shared path broke — assert_identifier_still_accepts_ordinary_names pins mt_* names, $, leading underscores, mixed case, non-ASCII letters, dashes and dotted partition suffixes as still valid.

The two items from this issue I did not take, with what I found:

The other providers' AssertValidIdentifier (listed as "not established"). Now established: SqlServerMigrator's is an empty method body (// Nothing yet), Oracle and MySql check length only, Sqlite checks null/whitespace and length. None of the four reject " or ;. Bringing them into line is a behaviour change per provider and deserves its own issue — happy to open one.

SchemaUtils.QuoteName and the rest of the quoting-helper table. QuoteName cannot simply double an embedded ": it only quotes at all when the name is a keyword or has an uppercase character, so a lowercase name carrying a quote is returned unquoted today, and the fix has to force quoting for those names too. That is the kind of change PostgresqlProvider.ToQualifiedName's pre-quoted-passthrough guard warns about — it is evidence that quote-wrapped names do circulate on that path, and re-quoting them would be a regression. Object names can no longer carry a quote past AssertValidIdentifier; what remains exposed through these helpers are schema names, index columns and collations, which do not route through it.