Skip to content

Fix .Update() marking AfterSaveBehavior.Throw properties as modified via DetectChanges#38115

Merged
AndriySvyryd merged 6 commits intomainfrom
copilot/update-not-mark-properties-as-modified
Apr 16, 2026
Merged

Fix .Update() marking AfterSaveBehavior.Throw properties as modified via DetectChanges#38115
AndriySvyryd merged 6 commits intomainfrom
copilot/update-not-mark-properties-as-modified

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

Fixes #38105

DbContext.Update() on an entity with a nullable complex type + discriminator throws during SaveChanges:

InvalidOperationException: The property 'X.Discriminator' is defined as read-only
after it has been saved, but its value has been modified or marked as modified.

Regression from #37394: the snapshot factory stores default values for shadow properties on complex types, so DetectChanges finds a false mismatch between the current discriminator value (set by value generator) and the snapshot (null), re-marking the Throw property as modified after SetEntityState correctly unflagged it.

Changes

  • InternalEntryBase.SetEntityState: In the modifyProperties block, after unflagging Throw properties, set their original values to match current values (guarded by !IsConceptualNull(property)) inline in the same loop so DetectChanges won't find a false mismatch and re-mark them as modified.

  • Test: Added Update_entity_with_nullable_complex_type_and_discriminator_does_not_throw — inserts entity with nullable complex type + discriminator, clears tracker, calls Update() with a new disconnected instance, asserts SaveChangesAsync succeeds.

A more targeted fix is proposed in #38111, but I believe this is a more robust approach

Copilot AI and others added 2 commits April 15, 2026 20:40
…s during Update() to prevent DetectChanges from re-marking them as modified

In SetEntityState with modifyProperties=true, after unflagging Throw properties, also ensure their original values match current values. This prevents DetectChanges from finding false mismatches due to snapshot factories storing default values for shadow properties on complex types.

Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/226e05b5-79be-4c42-9c60-b85c45bcf703

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Comment thread src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Outdated
Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/25d6f7dd-9b21-494f-af74-0354cf40a642

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copy link
Copy Markdown

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 regression where DbContext.Update() combined with DetectChanges can re-mark AfterSaveBehavior.Throw properties (notably the complex-type discriminator shadow property) as modified, causing SaveChanges to throw for optional complex types.

Changes:

  • Update InternalEntryBase.SetEntityState to synchronize original values for AfterSaveBehavior.Throw properties with current values to prevent false mismatches during DetectChanges.
  • Add a regression test covering Update() on an entity with a nullable complex type discriminator to ensure SaveChangesAsync succeeds.

Reviewed changes

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

File Description
src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Adjusts state-setting logic to prevent DetectChanges from re-marking Throw properties as modified due to snapshot/current mismatches.
test/EFCore.Specification.Tests/Query/AdHocComplexTypeQueryTestBase.cs Adds a regression test validating Update() + optional complex type discriminator no longer throws on save.

Comment thread src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Outdated
Comment thread src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Outdated
Comment thread src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Outdated
…move second pass

Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/920653c7-9690-47d4-8d9d-10857f722725

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copy link
Copy Markdown

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

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

…lex_type_and_discriminator_does_not_throw

Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/c4bbf4d0-14d6-4657-8eff-040197cda3bb

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested a review from AndriySvyryd April 15, 2026 23:10
@AndriySvyryd AndriySvyryd marked this pull request as ready for review April 15, 2026 23:15
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner April 15, 2026 23:15
Copilot AI review requested due to automatic review settings April 15, 2026 23:15
@AndriySvyryd AndriySvyryd assigned roji and unassigned AndriySvyryd and Copilot Apr 15, 2026
Copy link
Copy Markdown

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

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

Comment thread src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs Outdated
…Sql with actual SQL

Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/35584ae8-0472-4f4c-9a93-cdd770e3d1a3

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copy link
Copy Markdown
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

@AndriySvyryd is this something we need to service?

@roji roji assigned AndriySvyryd and unassigned roji Apr 16, 2026
@AndriySvyryd
Copy link
Copy Markdown
Member

is this something we need to service?

I think it's a corner case as we only had one report since 10.0.3

@AndriySvyryd AndriySvyryd enabled auto-merge (squash) April 16, 2026 06:53
@AndriySvyryd AndriySvyryd merged commit 3a6e825 into main Apr 16, 2026
15 checks passed
@AndriySvyryd AndriySvyryd deleted the copilot/update-not-mark-properties-as-modified branch April 16, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Complex type init only Discriminator mistakenly marked as modified when unchanged (regression)

4 participants