Skip to content

Migrations: Fix NPoco auto-select breaking retrust FK migration#22270

Merged
kjac merged 1 commit intomainfrom
v17/bugfix/fix-retrust-constraints
Mar 27, 2026
Merged

Migrations: Fix NPoco auto-select breaking retrust FK migration#22270
kjac merged 1 commit intomainfrom
v17/bugfix/fix-retrust-constraints

Conversation

@AndyButland
Copy link
Copy Markdown
Contributor

@AndyButland AndyButland commented Mar 26, 2026

Description

In 17.3 we've added a migration that re-trusts foreign key constraints that have become historically untrusted, primarily through use of SQL bulk inserts - see #21744.

We had #22227 raised against that - that it was attempting to work with non-Umbraco tables and also didn't correctly handle exceptions. This was fixed in #22229 and released in 17.3.0-rc2.

Unfortunately I didn't test that properly, as although it was verified to by-pass non-Umbraco tables, the fix introduced an issue with the actual re-trusting of constraints (in my test database, I no longer had any, so I didn't see it).

So this PR fixes that error.

You would see it with an install of 17.3.0-rc2 when you have a database containing untrusted constraints, with an exception thrown of An object or column name is missing or empty... Aliases defined as "" or [] are not allowed) in the RetrustForeignKeyAndCheckConstraints migration.

The cause is NPoco's AutoSelectHelper prepending SELECT ... FROM [] when raw SQL doesn't start with a recognized keyword (SELECT, EXEC, etc.). The TRY/CATCH block starts with BEGIN, which triggers auto-select against the empty [TableName("")] DTO attribute, producing invalid SQL.

To fix, a leading semicolon ; is used, which bypasses NPoco's auto-select.

Testing

Requires SQL Server (not SQLite).

  • Set up an untrusted constraint:
    ALTER TABLE [dbo].[umbracoContentSchedule] NOCHECK CONSTRAINT [FK_umbracoContentSchedule_umbracoLanguage_id];
    ALTER TABLE [dbo].[umbracoContentSchedule] CHECK CONSTRAINT [FK_umbracoContentSchedule_umbracoLanguage_id];
  • Verify it is untrusted:
    SELECT fk.name, fk.is_not_trusted
    FROM sys.foreign_keys fk
    WHERE fk.name = 'FK_umbracoContentSchedule_umbracoLanguage_id';
    -- Should show is_not_trusted = 1
  • Reset the migration state to re-run the re-trust step:
     UPDATE umbracoKeyValue
     SET value = '{B2F4A1C3-8D5E-4F6A-9B7C-3E1D2A4F5B6C}'
     WHERE [key] = 'Umbraco.Core.Upgrader.State+Umbraco.Core'
  • Start the application — the migration should complete without error
  • Verify the constraint is now trusted:
    SELECT fk.name, fk.is_not_trusted
    FROM sys.foreign_keys fk
    WHERE fk.name = 'FK_umbracoContentSchedule_umbracoLanguage_id';
    -- Should show is_not_trusted = 0

Copilot AI review requested due to automatic review settings March 26, 2026 12:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a SQL Server-only upgrade migration failure caused by NPoco auto-select misinterpreting a raw T-SQL BEGIN TRY block and generating invalid SQL.

Changes:

  • Prefixes the migration’s raw T-SQL with a leading semicolon to bypass NPoco AutoSelectHelper rewriting.
  • Documents the rationale inline, referencing the empty [TableName("")] DTO attributes that trigger the invalid FROM [] construction.

Copy link
Copy Markdown
Contributor

@kjac kjac left a comment

Choose a reason for hiding this comment

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

💪

@kjac kjac merged commit dd7fb87 into main Mar 27, 2026
32 checks passed
@kjac kjac deleted the v17/bugfix/fix-retrust-constraints branch March 27, 2026 08:17
AndyButland added a commit that referenced this pull request Mar 27, 2026
Prevent NPoco auto-select from breaking retrust migration.
@AndyButland
Copy link
Copy Markdown
Contributor Author

Cherry-picked to release/17.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants