Migrations: Re-trust untrusted foreign key and check constraints on SQL Server and fix bulk inserts to prevent recurrence

[AndyButland]

Prerequisites

• I have added steps to test this contribution in the description below

Description

What are untrusted constraints?

On SQL Server, foreign key and check constraints have a trust flag (is_not_trusted in sys.foreign_keys / sys.check_constraints). When a constraint is untrusted (is_not_trusted = 1), it means SQL Server has not validated that all existing rows in the table satisfy the constraint. This happens when constraints are added or re-enabled using NOCHECK, or when they are dropped and recreated on a populated table without WITH CHECK.

Why are untrusted constraints a problem?

Untrusted constraints have two significant downsides:

1. Query optimizer cannot use them. The SQL Server query optimizer relies on trusted constraints for join elimination, cardinality estimation, and index selection. When constraints are untrusted, the optimizer must assume the worst case, leading to suboptimal query plans and slower queries — particularly noticeable on large sites with many content nodes.

2. Data integrity is not guaranteed. An untrusted constraint means SQL Server enforces the rule for new/updated rows but has never verified that existing rows comply. There could be orphaned rows or invalid data that silently violates the constraint.

You can check for untrusted constraints on any SQL Server database with:

SELECT name, is_not_trusted FROM sys.foreign_keys WHERE is_not_trusted = 1;
SELECT name, is_not_trusted FROM sys.check_constraints WHERE is_not_trusted = 1;

Where do untrusted constraints come from?

It's not obvious looking at an upgrade from 13 to 17 where these come from.

Sites that have upgraded through multiple Umbraco major versions can carry untrusted foreign keys from the legacy CreateKeysAndIndexes migration, which dropped and recreated all FK constraints on populated tables without WITH CHECK. Other possible sources include external DBA operations such as backup restores or disable/enable constraint cycles.

I've had a couple of reports and see this in my local development database. I'm putting out some feelers to verify one a few real-world databases to see if the issue exists there (update: have heard back, it seems they do).

Update: It seems bulk inserts are the main culprit here. By default they use SqlBulkCopyOptions.Default, which disables constraints and marks them untrusted afterwards. We likely want to use the CheckConstraints option, at the potential performance cost on the inserts but for the benefit down the line for querying the data. So this is an extra task we need to consider and handle in this PR.

What this PR does

1. Makes FK creation on SQL Server explicitly use WITH CHECK — Overrides CreateForeignKeyConstraint in MicrosoftSqlSyntaxProviderBase to include WITH CHECK in the SQL template. This is already the SQL Server default for new constraints, so this is purely defensive to prevent any edge cases. Only affects SQL Server (SQLite doesn't inherit from this class).

2. Adds a migration that re-trusts all existing untrusted constraints — The RetrustForeignKeyConstraints migration queries sys.foreign_keys and sys.check_constraints for untrusted entries and runs ALTER TABLE ... WITH CHECK CHECK CONSTRAINT ... on each. It handles per-constraint failures gracefully (logs a warning and continues) so that data integrity violations don't block the upgrade. It skips SQLite entirely since SQLite has no trust concept.

3. Fixes Umbraco's own BulkInsertRecords to check constraints — Changes SqlServerBulkSqlInsertProvider to use SqlBulkCopyOptions.CheckConstraints instead of SqlBulkCopyOptions.Default when creating the SqlBulkCopy instance. This affects the 5 call sites that go through Database.BulkInsertRecords(): culture variation inserts during content save/publish, and content/media/member cache rebuilds.

4. Fixes NPoco's InsertBulk/InsertBulkAsync to check constraints — Introduces UmbracoSqlServerDatabaseType, a subclass of NPoco's SqlServer2012DatabaseType that overrides both InsertBulk and InsertBulkAsync to pass SqlBulkCopyOptions.CheckConstraints to SqlBulkCopyHelper. This is returned from SqlServerSyntaxProvider.GetUpdatedDatabaseType() so all ~20 call sites that use NPoco's bulk insert automatically get constraint checking with zero call-site changes. Affected operations include saving content property data, assigning user/group permissions, creating/updating webhooks, bulk saving relations, assigning member roles, saving external logins, saving document URLs, and adding distributed background jobs.

How to test

Migration

Automated: All existing migration unit tests continue to pass.

Manual on SQL Server:

1. See if you have any untrusted constraints.

   SELECT name, is_not_trusted FROM sys.foreign_keys WHERE is_not_trusted = 1;

2. If not, create some untrusted constraints on a test database:

   -- Disable and re-enable a constraint to make it untrusted
   ALTER TABLE [umbracoNode] NOCHECK CONSTRAINT ALL;
   ALTER TABLE [umbracoNode] CHECK CONSTRAINT ALL;
   -- Verify it's untrusted
   SELECT name, is_not_trusted FROM sys.foreign_keys WHERE is_not_trusted = 1;

3. Run the Umbraco upgrade (start the application so migrations execute)
4. Verify constraints are now trusted:

   SELECT name, is_not_trusted FROM sys.foreign_keys WHERE is_not_trusted = 1;
   -- Should return 0 rows (or only rows with genuine data integrity issues, logged as warnings)

Bulk Insert Behaviour

Carry out CMS operations like create content, update content, rebuild database index, create and update user group assignments to users, create and update user group granular permissions, create and update webhooks; and verify no untrusted foreign keys are found in the database.

SELECT name, is_not_trusted FROM sys.foreign_keys WHERE is_not_trusted = 1;

[Copilot]

Pull request overview

This PR improves SQL Server database integrity/performance during upgrades by ensuring foreign key constraints are created as trusted and by adding an upgrade migration to re-trust any existing untrusted FK/check constraints.

Changes:

• Add a new 17.3.0 upgrade migration that finds untrusted FK/check constraints and attempts to re-trust them.
• Register the new migration in the Umbraco upgrade plan for 17.3.0.
• Make SQL Server FK creation explicitly include WITH CHECK in the SQL syntax provider.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_3_0/RetrustForeignKeyConstraints.cs	New migration to locate untrusted constraints in SQL Server system catalogs and re-trust them (logging failures and continuing).
src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs	Wires the new migration into the 17.3.0 upgrade plan sequence.
src/Umbraco.Cms.Persistence.SqlServer/Services/MicrosoftSqlSyntaxProviderBase.cs	Updates SQL Server FK creation template to explicitly include WITH CHECK.

[kjac]

This works as advertised. I can approve and merge when you're ready, @AndyButland

One thought though. Are we in risk of SQL timeouts here, if the migration targets a huge table?

[AndyButland]

Are we in risk of SQL timeouts here, if the migration targets a huge table?

It's a good point. We can increase the timeout for this migration step, like we do in the one that converts the system dates, just in case.

[cornehoskam]

CC @ronaldbarendse into the conversation to take a look at this PR as well, as he's the one that informed me about this issue :)

[ronaldbarendse]

I can easily reproduce this on a clean CMS 17.1.0 installation with The Starter Kit, which returns the following untrusted foreign keys once the site is up and running:

FK_umbracoPropertyData_umbracoContentVersion_id
FK_umbracoPropertyData_cmsPropertyType_id
FK_umbracoPropertyData_umbracoLanguage_id
FK_umbracoRelation_umbracoNode
FK_umbracoRelation_umbracoNode1
FK_umbracoRelation_umbracoRelationType_id
FK_cmsContentNu_umbracoContent_nodeId
FK_umbracoDocumentUrl_umbracoNode_uniqueId
FK_umbracoDocumentUrl_umbracoLanguage_id

Just looking at the table names, these most likely used bulk inserts (either as part of the installation or data regeneration/seeding). And as already mentioned in the PR description, the default SqlBulkCopyOptions won't check constraints (see documentation).

I can also confirm that checking large tables can take a long time to complete (depending on the amount of rows, presence and state of indexes and available resources), so I'd avoid running this as part of a migration. I'd suggest only fixing the cause of untrusted constraints (most likely only changing SqlBulkCopyOptions.Default to SqlBulkCopyOptions.CheckConstraints in SqlServerBulkSqlInsertProvider) and optionally creating a Health Check that identifies the untrusted (and potentially disabled) constraints. This health check can even have actions to execute the ...WITH CHECK CHECK CONSTRAINT... SQL commands from the backoffice, although the same timeout (and locking) issues will need to be considered.

[AndyButland]

I tested this on a database with 1,558,275 records in umbracoPropertyData and the three executions on that table only take 250-400ms each. The whole migration for the database completes in 4s.

That's a sample size of 1 of course but suggests to me it should be timely enough to run in an migration with an execution timeout of 300s as we have now. Given this will seemingly exist in any Umbraco database created before 17.3, I think it's worth getting it fixed up once and for all. Otherwise it's an education task for a lot of Umbraco developers to understand what these are, what the impact is, and how to manually resolve them (I'll confess, I'd never heard of them until last week!).

[kjac]

@AndyButland I've approved it now. Feel free to merge it in when you see fit 👍