Issue #31819: Fast DetectChanges; Issue #16491: legacy Merge-Option Feature by DamirLisak · Pull Request #37812 · dotnet/efcore

DamirLisak · 2026-02-27T12:00:05Z

Issue #31819 "DetectChanges slow in long-term contexts" is no longer necessary if all entity classes in the model are defined with a change-tracking strategy other than "ChangeTrackingStrategy.Snapshot." This is controlled in the ProcessModelFinalizing method of the ChangeTrackingStrategyConvention class. DetectChanges is therefore not called when saving because change tracking is performed using the EntityReferenceMap (_entityReferenceMap property), which essentially corresponds to the previous caching mechanism using EntityState dictionaries from EF4/5/6. However, querying this cache was not enabled; instead, users were forced to use ChangeTracker.Entries(), which returned all objects and led to performance problems with large long-term contexts. This change now enables fast access to changed objects using the new GetEntriesForState method in the ChangeTracker class. This, in turn, calls EntityReferenceMap.GetEntriesForState(), which already exists.
Added test for testing ChangeTracker.GetEntries(EntityState)

Issue #16491: Implemented legacy Merge-Option Feature from EF/4/5/6
Added test for testing the Merge-Option Feature

I've read the guidelines for contributing and seen the walkthrough
I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
The code builds and tests pass locally (also verified by our automated build checks)
Commit messages follow this format:

        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber

Tests for the changes have been added (for bug fixes / features)
Code follows the same patterns and style as existing code in this repo

Replaces #36556

…onger necessary if all entity classes in the model are defined with a change-tracking strategy other than "ChangeTrackingStrategy.Snapshot." This is controlled in the ProcessModelFinalizing method of the ChangeTrackingStrategyConvention class. DetectChanges is therefore not called when saving because change tracking is performed using the EntityReferenceMap (_entityReferenceMap property), which essentially corresponds to the previous caching mechanism using EntityState dictionaries from EF4/5/6. However, querying this cache was not enabled; instead, users were forced to use ChangeTracker.Entries(), which returned all objects and led to performance problems with large long-term contexts. This change now enables fast access to changed objects using the new GetEntriesForState method in the ChangeTracker class. This, in turn, calls EntityReferenceMap.GetEntriesForState(), which already exists. For fast change tracking, implement INotifyProperty-Changed on your entity classes and activate this strategy in the Model Builder using ChangeTrackingStrategy.ChangedNotifications via HasChangeTrackingStrategy(). dotnet#31819 Added test for testing ChangeTracker.GetEntries(EntityState)

Copilot

Pull request overview

This PR aims to address two issues: #31819 (fast DetectChanges for long-term contexts) and #16491 (legacy Merge-Option feature from EF4/5/6). The implementation adds a new MergeOption enum and Refresh() query extension method that allows refreshing tracked entities with database values, plus new GetEntriesForState() methods on ChangeTracker for fast access to entities by state.

Changes:

Adds public API for refreshing entities: MergeOption enum, Refresh() extension method, and overloads for EntityEntry.Reload()
Adds ChangeTracker.GetEntriesForState() methods for fast filtered access to tracked entities
Modifies query compilation pipeline to support merge options during entity materialization
Adds comprehensive test suite for the Merge-Option feature (but not for GetEntriesForState)

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
src/EFCore/MergeOption.cs	New enum defining merge behavior (AppendOnly, OverwriteChanges, PreserveChanges)
src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs	Adds Refresh() extension method to enable merge options on queries
src/EFCore/ChangeTracking/ChangeTracker.cs	Adds GetEntriesForState() methods for fast state-based entity filtering
src/EFCore/ChangeTracking/EntityEntry.cs	Adds Reload() overloads accepting MergeOption parameter
src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs	Adds ReloadValue() internal method implementing property-level refresh logic
src/EFCore/Query/QueryCompilationContext.cs	Adds RefreshMergeOption property to track merge behavior during compilation
src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs	Handles Refresh() method call normalization
src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs	Implements merge logic during entity materialization
src/EFCore/Properties/CoreStrings.resx	Adds error messages for Refresh validation
src/EFCore/Properties/CoreStrings.Designer.cs	Generated error message accessors
test/EFCore.Specification.Tests/MergeOptionTestBase.cs	Comprehensive test base class for merge options across scenarios
test/EFCore.Sqlite.FunctionalTests/Query/MergeOptionSqliteTest.cs	SQLite-specific test implementation
test/EFCore.SqlServer.FunctionalTests/Query/MergeOptionSqlServerTest.cs	SQL Server-specific test implementation

Files not reviewed (1)

src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported

test/EFCore.Sqlite.FunctionalTests/Query/MergeOptionSqliteTest.cs

test/EFCore.SqlServer.FunctionalTests/Query/MergeOptionSqlServerTest.cs

Copilot · 2026-02-27T12:08:59Z

src/EFCore/ChangeTracking/ChangeTracker.cs

+    /// <summary>
+    /// Returns tracked entities that are in a given state from a fast cache.
+    /// </summary>
+    /// <param name="added">Entities in EntityState.Added state</param>
+    /// <param name="modified">Entities in Modified.Added state</param>
+    /// <param name="deleted">Entities in Modified.Deleted state</param>
+    /// <param name="unchanged">Entities in Modified.Unchanged state</param>
+    /// <returns>An entry for each entity that matched the search criteria.</returns>
+    public virtual IEnumerable<EntityEntry> GetEntriesForState(
+        bool added = false,
+        bool modified = false,
+        bool deleted = false,
+        bool unchanged = false)
+    {
+        return StateManager.GetEntriesForState(added, modified, deleted, unchanged)
+            .Select(e => new EntityEntry(e));
+    }


The documentation for this method is incomplete and doesn't follow EF Core documentation conventions. It should include:

A proper summary describing what the method does and when to use it

Remarks section with links to documentation using <see href="https://aka.ms/efcore-docs-*"> format

Proper parameter descriptions using <param> tags

Better description of the state parameters (currently says "Modified.Added" and "Modified.Deleted" which appear to be copy-paste errors - should be "EntityState.Modified" and "EntityState.Deleted")

Compare with the existing Entries() method documentation at lines 183-216 for the expected format.

src/EFCore/Query/QueryCompilationContext.cs

Copilot · 2026-02-27T12:08:59Z

src/EFCore/ChangeTracking/EntityEntry.cs

+    private void Reload(PropertyValues? storeValues, MergeOption mergeOption)
+    {
+        if (storeValues == null)
+        {
+            if (State != EntityState.Added)
+            {
+                State = EntityState.Deleted;
+                State = EntityState.Detached;
+            }
+        }
+        else
+        {
+            foreach (var property in Metadata.GetProperties())
+            {
+                var value = storeValues[property];
+                InternalEntry.ReloadValue(property, value, mergeOption, updateEntityState: false);
+            }
+
+            if (mergeOption == MergeOption.OverwriteChanges)
+            {
+                State = EntityState.Unchanged;
+            }
+        }
+    }


The Reload(PropertyValues, MergeOption) method only iterates over properties (line 771) but doesn't handle navigations. The existing Reload(PropertyValues) method (lines 742-758) uses PropertyValues.SetValues() which handles both properties and navigations. Consider whether navigations should also be refreshed, particularly for owned entities and complex types. If navigations should not be refreshed, this should be documented in the method's XML comments.

Copilot · 2026-02-27T12:09:01Z

src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs

+    public virtual void ReloadValue(IPropertyBase propertyBase, object? value, MergeOption mergeOption, bool updateEntityState)
+    {
+        var property = (IProperty)propertyBase;
+        EnsureOriginalValues();
+        bool isModified = IsModified(property);
+        _originalValues.SetValue(property, value, -1);
+        if (mergeOption == MergeOption.OverwriteChanges || !isModified)
+            SetProperty(propertyBase, value, isMaterialization: true, setModified: false);
+        if (updateEntityState)
+        {
+            if (mergeOption == MergeOption.OverwriteChanges)
+                SetEntityState(EntityState.Unchanged);
+            else
+                ((StateManager as StateManager)?.ChangeDetector as ChangeDetector)?.DetectValueChange(this, property);
+        }
+    }


The ReloadValue method calls ChangeDetector.DetectValueChange for individual properties (line 1021), but this may be inefficient when reloading many properties. When updateEntityState is true and mergeOption is PreserveChanges, DetectValueChange is called for every property individually. Consider batching these calls or using a more efficient approach, especially since this is in the hot path during query materialization with Refresh.

Copilot · 2026-02-27T12:09:01Z

src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs

+    /// <summary>
+    /// Specifies that the current Entity Framework LINQ query should refresh already loaded objects with the specified merge option.
+    /// </summary>
+    /// <typeparam name="T">The type of entity being queried.</typeparam>
+    /// <param name="source">The source query.</param>
+    /// <param name="mergeOption">The MergeOption</param>
+    /// <returns>A new query annotated with the given tag.</returns>
+    public static IQueryable<T> Refresh<T>(
+        this IQueryable<T> source,
+        [NotParameterized] MergeOption mergeOption)
+    {
+        if (HasNonTrackingOrIgnoreAutoIncludes(source.Expression))
+        {
+            throw new InvalidOperationException(CoreStrings.RefreshNonTrackingQuery);
+        }
+
+        if (HasMultipleMergeOptions(source.Expression))
+        {
+            throw new InvalidOperationException(CoreStrings.RefreshMultipleMergeOptions);
+        }
+        return
+            source.Provider is EntityQueryProvider
+                ? source.Provider.CreateQuery<T>(
+                    Expression.Call(
+                        instance: null,
+                        method: RefreshMethodInfo.MakeGenericMethod(typeof(T)),
+                        arg0: source.Expression,
+                        arg1: Expression.Constant(mergeOption)))
+                : source;
+    }


The documentation for the Refresh method is incomplete and doesn't follow EF Core documentation conventions. It should include:

A proper remarks section explaining how this method interacts with the change tracker and when to use it

Links to documentation using the <see href="https://aka.ms/efcore-docs-*"> format (not hardcoded learn.microsoft.com URLs)

An example showing how to use this method

Better parameter documentation - the current "The MergeOption" description is not helpful

The return value description says "A new query annotated with the given tag" which is incorrect (copy-pasted from another method) - should describe that it returns a query that will refresh tracked entities when materialized

Copilot · 2026-02-27T12:09:01Z

src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs

+            // Get all properties to update (exclude key properties which should not change)
+            var propertiesToUpdate = entityType.GetProperties()
+                .Where(p => !p.IsPrimaryKey())
+                .ToList();
+
+            var setReloadValueMethod = typeof(InternalEntityEntry)
+                .GetMethod(nameof(InternalEntityEntry.ReloadValue), new[] { typeof(IPropertyBase), typeof(object), typeof(MergeOption), typeof(bool) })!;
+
+            // Update original values similar to EntityEntry.Reload()
+            // This ensures that the original values snapshot reflects the database state
+            var dbProperties = propertiesToUpdate;
+            int count = dbProperties.Count();
+            int i = 0;
+            foreach (var property in dbProperties)
+            {
+                i++;
+                var newValue = valueBufferExpression.CreateValueBufferReadValueExpression(
+                    property.ClrType,
+                    property.GetIndex(),
+                    property);
+
+                var setOriginalValueExpression = Call(
+                    entryVariable,
+                    setReloadValueMethod,
+                    Constant(property),
+                    property.ClrType.IsValueType && property.IsNullable
+                        ? (Expression)Convert(newValue, typeof(object))
+                        : Convert(newValue, typeof(object)),
+                    Constant(_mergeOption),
+                    Constant(i == count));
+
+                updateExpressions.Add(setOriginalValueExpression);
+            }
+
+            return updateExpressions.Count > 0
+                ? (Expression)Block(updateExpressions)
+                : Empty();
+        }


The UpdateExistingEntityWithDatabaseValues method only excludes primary key properties when refreshing entity values, but it should also consider:

Foreign key properties that shouldn't change

Properties with ValueGenerated.OnAdd that shouldn't be updated after creation

Concurrency tokens which have special handling

Consider using a pattern similar to the existing Reload() method in EntityEntry which has proper logic for determining which properties should be updated. Also consider whether shadow properties should be handled differently.

test/EFCore.Specification.Tests/MergeOptionTestBase.cs

Copilot · 2026-02-27T12:09:02Z

src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs

+                                // Update the existing entity with new property values from the database
+                                // if the merge option is not AppendOnly
+                                _mergeOption != MergeOption.AppendOnly
+                                ? UpdateExistingEntityWithDatabaseValues(
+                                    entryVariable,
+                                    concreteEntityTypeVariable,
+                                    materializationContextVariable,
+                                    shaper) : Empty()),


The UpdateExistingEntityWithDatabaseValues method is called every time an entity is found in the change tracker (lines 533-538), even when MergeOption is AppendOnly. While there's a check _mergeOption != MergeOption.AppendOnly, this creates unnecessary expression tree nodes. Consider checking the merge option earlier to avoid creating the conditional expression entirely when not needed, or restructure the logic to be more efficient.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 20 comments.

Files not reviewed (1)

src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported

Copilot · 2026-02-27T14:28:50Z

src/EFCore/Properties/CoreStrings.Designer.cs

+        ///     Unable to refresh when is not tracked query
+        /// </summary>
+        public static string RefreshNonTrackingQuery
+            => GetString("RefreshNonTrackingQuery");
+
+        /// <summary>
+        ///     Merge option changed on same query


The XML doc summary for this resource property is ungrammatical and doesn't match the actual resource message in the .resx. Since this is a public surface for providers/extensions, update the summary (or regenerate the designer) to avoid misleading documentation.

Suggested change

/// Unable to refresh when is not tracked query

/// </summary>

public static string RefreshNonTrackingQuery

=> GetString("RefreshNonTrackingQuery");

/// <summary>

/// Merge option changed on same query

/// Unable to refresh because the query is not being tracked.

/// </summary>

public static string RefreshNonTrackingQuery

=> GetString("RefreshNonTrackingQuery");

/// <summary>

/// The merge option was changed on the same query.

Copilot · 2026-02-27T14:28:50Z

src/EFCore/ChangeTracking/ChangeTracker.cs

+    /// <param name="modified">Entities in Modified.Added state</param>
+    /// <param name="deleted">Entities in Modified.Deleted state</param>
+    /// <param name="unchanged">Entities in Modified.Unchanged state</param>


The XML doc parameter descriptions for modified, deleted, and unchanged reference non-existent states like "Modified.Added"/"Modified.Deleted". These should refer to EntityState.Modified, EntityState.Deleted, and EntityState.Unchanged (and ideally use <see cref="EntityState.*" />).

Suggested change

/// <param name="modified">Entities in Modified.Added state</param>

/// <param name="deleted">Entities in Modified.Deleted state</param>

/// <param name="unchanged">Entities in Modified.Unchanged state</param>

/// <param name="modified">Entities in the <see cref="EntityState.Modified" /> state.</param>

/// <param name="deleted">Entities in the <see cref="EntityState.Deleted" /> state.</param>

/// <param name="unchanged">Entities in the <see cref="EntityState.Unchanged" /> state.</param>

Copilot · 2026-02-27T14:28:50Z

src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs

+        if (HasNonTrackingOrIgnoreAutoIncludes(source.Expression))
+        {
+            throw new InvalidOperationException(CoreStrings.RefreshNonTrackingQuery);
+        }


Refresh relies on HasNonTrackingOrIgnoreAutoIncludes(source.Expression) to enforce tracking. This only catches explicit AsNoTracking*() calls; it won't catch queries that are non-tracking because ChangeTracker.QueryTrackingBehavior is configured globally, making Refresh a silent no-op instead of throwing.

Copilot · 2026-02-27T14:28:51Z

src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs

+        private Expression UpdateExistingEntityWithDatabaseValues(
+            ParameterExpression entryVariable,
+            ParameterExpression concreteEntityTypeVariable,
+            ParameterExpression materializationContextVariable,
+            StructuralTypeShaperExpression shaper)


UpdateExistingEntityWithDatabaseValues takes concreteEntityTypeVariable, but it isn't used. If the intent is to refresh values based on the concrete runtime type (e.g. to include derived-type properties when querying a base set), the current implementation won't do that. Either remove the parameter or use it to ensure the refreshed property set matches the concrete entity type.

Copilot · 2026-02-27T14:28:51Z

src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs

+                    || name == nameof(AsNoTrackingWithIdentityResolution)
+                    || name == nameof(IgnoreAutoIncludes))


HasNonTrackingOrIgnoreAutoIncludes treats IgnoreAutoIncludes() as equivalent to non-tracking and causes Refresh to throw RefreshNonTrackingQuery. Ignoring auto-includes doesn't disable tracking, so this check (or the exception message) should be adjusted.

Suggested change

|| name == nameof(AsNoTrackingWithIdentityResolution)

|| name == nameof(IgnoreAutoIncludes))

|| name == nameof(AsNoTrackingWithIdentityResolution))

Copilot · 2026-02-27T14:28:53Z

src/EFCore/Properties/CoreStrings.Designer.cs

                entityType);

+        /// <summary>
+        ///     Unable to refresh when is not tracked query


The XML doc summary for this resource property is ungrammatical and doesn't match the actual resource message in the .resx. Since this is a public surface for providers/extensions, update the summary (or regenerate the designer) to avoid misleading documentation.

Suggested change

/// Unable to refresh when is not tracked query

/// Message indicating that a query cannot be refreshed because it is not being tracked.

Copilot · 2026-02-27T14:28:53Z

src/EFCore/ChangeTracking/ChangeTracker.cs

+        bool modified = false,
+        bool deleted = false,
+        bool unchanged = false)
+    {


GetEntriesForState doesn't call TryDetectChanges() (unlike Entries()/Entries<TEntity>() above). With snapshot tracking this can return stale results unless DetectChanges() was run elsewhere. Either call TryDetectChanges() here or document explicitly that this is a "no DetectChanges" fast-path and may not reflect up-to-date state.

Suggested change

{

{

TryDetectChanges();

Copilot · 2026-02-27T14:28:54Z

test/EFCore.Specification.Tests/MergeOptionTestBase.cs

+        UseTransaction(context, ctx =>
+        {
+            var product = ctx.Set<Product>().First();
+            var originalTags = product.Tags.ToList();


Unused local originalTags will produce CS0219 (treated as error because TreatWarningsAsErrors=true). Remove it or use it in an assertion.

Suggested change

var originalTags = product.Tags.ToList();

Copilot · 2026-02-27T14:28:54Z

test/EFCore.Specification.Tests/MergeOptionTestBase.cs

+        UseTransaction(context, ctx =>
+        {
+            var product = ctx.Set<Product>().First();
+            var currentName = product.Name;


Unused local currentName will produce CS0219 (treated as error because TreatWarningsAsErrors=true). Remove it or use it in an assertion.

Suggested change

var currentName = product.Name;

Copilot · 2026-02-27T14:28:54Z

src/EFCore/ChangeTracking/ChangeTracker.cs

+    public virtual IEnumerable<EntityEntry> GetEntriesForState(
+        bool added = false,
+        bool modified = false,
+        bool deleted = false,
+        bool unchanged = false)


No tests were found covering the new public ChangeTracker.GetEntriesForState* APIs. Adding tests for each state flag combination (and for behavior when AutoDetectChangesEnabled is true/false) would help prevent regressions, especially since this method intentionally differs from Entries() behavior.

roji · 2026-02-27T14:52:23Z

@AndriySvyryd let me know if you want us to discuss this in design.

DamirLisak requested a review from a team as a code owner February 27, 2026 12:00

Copilot AI review requested due to automatic review settings February 27, 2026 12:00

github-actions bot added the community-contribution label Feb 27, 2026

Copilot started reviewing on behalf of DamirLisak February 27, 2026 12:00 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Apply suggestions from code review

9ea9609

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 27, 2026 14:16

Copilot started reviewing on behalf of DamirLisak February 27, 2026 14:17 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

roji assigned AndriySvyryd Feb 27, 2026

		\|\| name == nameof(AsNoTrackingWithIdentityResolution)
		\|\| name == nameof(IgnoreAutoIncludes))

	/// Unable to refresh when is not tracked query
	/// Message indicating that a query cannot be refreshed because it is not being tracked.

Conversation

DamirLisak commented Feb 27, 2026 • edited by AndriySvyryd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

roji commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DamirLisak commented Feb 27, 2026 •

edited by AndriySvyryd

Loading