Fold PostgresqlMigrator.AssertValidIdentifier onto the shared helper (weasel#416) - #422
Merged
Merged
Conversation
…er (weasel#416)
The previous commits left PostgreSQL on its own hand-rolled loop on the grounds
that folding it in would be a no-behaviour-change edit to code shipped in 9.23.0.
That was half right: the loop is now IdentifierValidation.FindProblem, and the
rules it had -- null/empty/whitespace-only, whitespace anywhere, '"', ';' -- come
back identical, with the same reason strings, so PostgresqlIdentifierInvalidException
.Reason and its message are unchanged. The length check and
PostgresqlIdentifierTooLongException stay here, since NameDataLength is
PostgreSQL's own and configurable.
The behaviour delta is the single quote, which the shared helper rejects for every
provider and PostgreSQL did not. It is not uniformity for its own sake here either:
Table.ColumnExpression.DefaultValueFromSequence writes a sequence's name into a
string literal -- DEFAULT nextval('{name}') -- so a name carrying a quote closes
that literal and the rest of the DDL is whatever the name says next. PostgreSQL's
introspection queries parameterise names (Table.FetchExisting, Function), which is
why the exposure is narrower here than on the providers that interpolate them.
An apostrophe in a schema object name has no legitimate use that survives Weasel's
quoting anyway -- SchemaUtils.QuoteName only quotes keywords and names with an
uppercase character, so such a name is written out raw -- but it is a tightening on
top of a released one, and worth reading as such.
Full Weasel.Postgresql suite green (849 passed, 3 pre-existing skips -- 846 before,
plus the three single-quote cases added to assert_identifier_rejects_quote_and_semicolon).
Weasel.Core 75, Weasel.EntityFrameworkCore 106 passed/1 skip, Weasel.CommandLine 23.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Follow-on to #420, which left PostgreSQL on its own hand-rolled loop — the reasoning there was that folding it in would be a no-behaviour-change edit to code shipped in 9.23.0. That turns out to be half right, so the delta is worth stating plainly up front.
Unchanged: the rules PostgreSQL already had come back identical — null/empty/whitespace-only, whitespace anywhere,
",;— with the same reason strings, soPostgresqlIdentifierInvalidException.Reasonand the exception message are byte-for-byte what they were. The length check andPostgresqlIdentifierTooLongExceptionstay in the provider, sinceNameDataLengthis PostgreSQL's own and configurable.Changed: the shared helper also rejects
', and PostgreSQL did not.That is not uniformity for its own sake. Table.cs:584 —
DefaultValueFromSequence— writes a sequence'sDbObjectNameinto a string literal:so a sequence name carrying a
'closes that literal and the rest of theDEFAULTclause is whatever the name says next. PostgreSQL's introspection queries do parameterise names (Table.FetchExisting,Function), which is why the exposure here is narrower than on SQL Server, Oracle and SQLite, where the existence checks interpolate them — but it is not zero.An apostrophe in a schema object name has no legitimate use that survives Weasel's quoting anyway:
SchemaUtils.QuoteNameonly quotes keywords and names containing an uppercase character, so such a name is written out raw. Still, it is a tightening layered on a released one, and anyone with a'in an object name will now get an exception where 9.23.0 gave them DDL.The three single-quote cases are added to
assert_identifier_rejects_quote_and_semicolonrather than to a new test, since it is the same rule.Regression pass
Full suites on net9.0, PostgreSQL container from
docker compose up:The other four providers are untouched by this — their behaviour landed in #420.
🤖 Generated with Claude Code