From 48c957f366ecc2aa00f984e436fae4a70f54b9d1 Mon Sep 17 00:00:00 2001 From: Ladislav Bitto Date: Wed, 12 Nov 2025 09:24:00 -0800 Subject: [PATCH 1/3] Fixing named query cache misses by adding support for collections in constant expression comparer --- .../EntityFrameworkQueryableExtensions.cs | 3 +- .../Query/ExpressionEqualityComparer.cs | 51 ++++++++++++++++++- .../Query/AdHocQueryFiltersQueryTestBase.cs | 22 ++++++++ .../Query/ExpressionEqualityComparerTest.cs | 36 +++++++++++++ 4 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs b/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs index 0602cf9ea5b..21a6cb73270 100644 --- a/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs +++ b/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs @@ -2710,7 +2710,8 @@ public static IQueryable IgnoreQueryFilters( Expression.Call( instance: null, method: IgnoreNamedQueryFiltersMethodInfo.MakeGenericMethod(typeof(TEntity)), - arguments: [source.Expression, Expression.Constant(filterKeys)])) + // converting the collection to a sorted set to ensure cache is hit even when order of keys differs + arguments: [source.Expression, Expression.Constant(new SortedSet(filterKeys))])) : source; #endregion diff --git a/src/EFCore/Query/ExpressionEqualityComparer.cs b/src/EFCore/Query/ExpressionEqualityComparer.cs index 3f2875f866f..1aabc713f27 100644 --- a/src/EFCore/Query/ExpressionEqualityComparer.cs +++ b/src/EFCore/Query/ExpressionEqualityComparer.cs @@ -76,6 +76,13 @@ public int GetHashCode(Expression obj) hash.Add(structuralEquatable.GetHashCode(StructuralComparisons.StructuralEqualityComparer)); break; + case IEnumerable enumerable when constantExpression.Value is not string: + foreach (var item in enumerable) + { + hash.Add(item); + } + break; + default: hash.Add(constantExpression.Value); break; @@ -368,8 +375,48 @@ private static bool CompareConstant(ConstantExpression a, ConstantExpression b) { var (v1, v2) = (a.Value, b.Value); - return Equals(v1, v2) - || (v1 is IStructuralEquatable array1 && array1.Equals(v2, StructuralComparisons.StructuralEqualityComparer)); + if (Equals(v1, v2)) + { + return true; + } + + if (v1 is IStructuralEquatable array1) + { + return array1.Equals(v2, StructuralComparisons.StructuralEqualityComparer); + } + + // Handle collections that don't implement IStructuralEquatable + if (v1 is IEnumerable enumerable1 && v1 is not string + && v2 is IEnumerable enumerable2 && v2 is not string) + { + return SequenceEqual(enumerable1, enumerable2); + } + + return false; + } + + private static bool SequenceEqual(IEnumerable x, IEnumerable y) + { + var xEnumerator = x.GetEnumerator(); + var yEnumerator = y.GetEnumerator(); + + try + { + while (xEnumerator.MoveNext()) + { + if (!yEnumerator.MoveNext() || !Equals(xEnumerator.Current, yEnumerator.Current)) + { + return false; + } + } + + return !yEnumerator.MoveNext(); + } + finally + { + (xEnumerator as IDisposable)?.Dispose(); + (yEnumerator as IDisposable)?.Dispose(); + } } private bool CompareGoto(GotoExpression a, GotoExpression b) diff --git a/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs index 412b9508dd2..31854f7a344 100644 --- a/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs @@ -34,6 +34,28 @@ public virtual async Task Named_query_filters_ignore_some() Assert.Equal(2, result.Count); } + [ConditionalFact] + public virtual async Task Named_query_filters_caching() + { + var cacheLog = new List(); + var contextFactory = await InitializeAsync(seed: c => c.SeedAsync(), onConfiguring: builder => + { + builder.EnableSensitiveDataLogging(); + builder.LogTo(cacheLog.Add, filter: (eventid, _) => eventid.Name == CoreEventId.QueryCompilationStarting.Name); + }); + using var context = contextFactory.CreateContext(); + _ = context.Entities + .IgnoreQueryFilters(["ActiveFilter", "NameFilter"]) + .ToList(); + _ = context.Entities + .IgnoreQueryFilters(["ActiveFilter", "NameFilter"]) + .ToList(); + _ = context.Entities + .IgnoreQueryFilters(["NameFilter", "ActiveFilter"]) + .ToList(); + Assert.Equal(1, cacheLog.Count); + } + [ConditionalFact] public virtual async Task Named_query_filters_ignore_all() { diff --git a/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs b/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs index 8aa4bfe7b67..ec129dd4ee0 100644 --- a/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs +++ b/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs @@ -107,6 +107,42 @@ public void Array_constant_expressions_are_compared_correctly() Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e3)); } + [ConditionalFact] + public void Collection_constant_expressions_are_compared_correctly() + { + var expressionComparer = ExpressionEqualityComparer.Instance; + + var e1 = Constant(new List { "value1", "value2" }); + var e2 = Constant(new List { "value1", "value2" }); + var e3 = Constant(new List { "value1", "value3" }); + var e4 = Constant(new List { "value2", "value1" }); + + Assert.True(expressionComparer.Equals(e1, e2)); + Assert.False(expressionComparer.Equals(e1, e3)); + Assert.False(expressionComparer.Equals(e1, e4)); + + Assert.Equal(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e2)); + Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e3)); + Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e4)); + } + + [ConditionalFact] + public void Sorted_collection_constant_expressions_with_different_insertion_order_are_equal() + { + var expressionComparer = ExpressionEqualityComparer.Instance; + + // Test sorted collections with elements in different order + var e1 = Constant(new SortedSet { "value2", "value1", "value3" }); + var e2 = Constant(new SortedSet { "value1", "value3", "value2" }); + var e3 = Constant(new SortedSet { "value1", "value2", "value4" }); + + Assert.True(expressionComparer.Equals(e1, e2)); + Assert.False(expressionComparer.Equals(e1, e3)); + + Assert.Equal(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e2)); + Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e3)); + } + [ConditionalFact] // #30697 public void Lambda_parameters_names_are_taken_into_account() { From d353e2fc048a1f77ebae1b32a80cffd70bbec1ae Mon Sep 17 00:00:00 2001 From: Ladislav Bitto Date: Thu, 20 Nov 2025 08:32:28 -0800 Subject: [PATCH 2/3] Addressing comments In PR --- .../EntityFrameworkQueryableExtensions.cs | 4 +- .../Query/ExpressionEqualityComparer.cs | 51 +------------------ .../Query/AdHocQueryFiltersQueryTestBase.cs | 5 +- .../Query/ExpressionEqualityComparerTest.cs | 36 ------------- 4 files changed, 8 insertions(+), 88 deletions(-) diff --git a/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs b/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs index 21a6cb73270..c6ce2d2b44f 100644 --- a/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs +++ b/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs @@ -2710,8 +2710,8 @@ public static IQueryable IgnoreQueryFilters( Expression.Call( instance: null, method: IgnoreNamedQueryFiltersMethodInfo.MakeGenericMethod(typeof(TEntity)), - // converting the collection to a sorted set to ensure cache is hit even when order of keys differs - arguments: [source.Expression, Expression.Constant(new SortedSet(filterKeys))])) + // converting the collection to an array if it isn't already one to ensure consistent caching + arguments: [source.Expression, Expression.Constant(filterKeys is string[] ? filterKeys : filterKeys.ToArray())])) : source; #endregion diff --git a/src/EFCore/Query/ExpressionEqualityComparer.cs b/src/EFCore/Query/ExpressionEqualityComparer.cs index 1aabc713f27..3f2875f866f 100644 --- a/src/EFCore/Query/ExpressionEqualityComparer.cs +++ b/src/EFCore/Query/ExpressionEqualityComparer.cs @@ -76,13 +76,6 @@ public int GetHashCode(Expression obj) hash.Add(structuralEquatable.GetHashCode(StructuralComparisons.StructuralEqualityComparer)); break; - case IEnumerable enumerable when constantExpression.Value is not string: - foreach (var item in enumerable) - { - hash.Add(item); - } - break; - default: hash.Add(constantExpression.Value); break; @@ -375,48 +368,8 @@ private static bool CompareConstant(ConstantExpression a, ConstantExpression b) { var (v1, v2) = (a.Value, b.Value); - if (Equals(v1, v2)) - { - return true; - } - - if (v1 is IStructuralEquatable array1) - { - return array1.Equals(v2, StructuralComparisons.StructuralEqualityComparer); - } - - // Handle collections that don't implement IStructuralEquatable - if (v1 is IEnumerable enumerable1 && v1 is not string - && v2 is IEnumerable enumerable2 && v2 is not string) - { - return SequenceEqual(enumerable1, enumerable2); - } - - return false; - } - - private static bool SequenceEqual(IEnumerable x, IEnumerable y) - { - var xEnumerator = x.GetEnumerator(); - var yEnumerator = y.GetEnumerator(); - - try - { - while (xEnumerator.MoveNext()) - { - if (!yEnumerator.MoveNext() || !Equals(xEnumerator.Current, yEnumerator.Current)) - { - return false; - } - } - - return !yEnumerator.MoveNext(); - } - finally - { - (xEnumerator as IDisposable)?.Dispose(); - (yEnumerator as IDisposable)?.Dispose(); - } + return Equals(v1, v2) + || (v1 is IStructuralEquatable array1 && array1.Equals(v2, StructuralComparisons.StructuralEqualityComparer)); } private bool CompareGoto(GotoExpression a, GotoExpression b) diff --git a/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs index 31854f7a344..c84d80fef18 100644 --- a/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs @@ -53,7 +53,10 @@ public virtual async Task Named_query_filters_caching() _ = context.Entities .IgnoreQueryFilters(["NameFilter", "ActiveFilter"]) .ToList(); - Assert.Equal(1, cacheLog.Count); + + // #37212 - ExpressionEqualityComparer doesn't support collections besides an array, + // therefore we can't implement caching for different order of ignored filters + Assert.Equal(2, cacheLog.Count); } [ConditionalFact] diff --git a/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs b/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs index ec129dd4ee0..8aa4bfe7b67 100644 --- a/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs +++ b/test/EFCore.Tests/Query/ExpressionEqualityComparerTest.cs @@ -107,42 +107,6 @@ public void Array_constant_expressions_are_compared_correctly() Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e3)); } - [ConditionalFact] - public void Collection_constant_expressions_are_compared_correctly() - { - var expressionComparer = ExpressionEqualityComparer.Instance; - - var e1 = Constant(new List { "value1", "value2" }); - var e2 = Constant(new List { "value1", "value2" }); - var e3 = Constant(new List { "value1", "value3" }); - var e4 = Constant(new List { "value2", "value1" }); - - Assert.True(expressionComparer.Equals(e1, e2)); - Assert.False(expressionComparer.Equals(e1, e3)); - Assert.False(expressionComparer.Equals(e1, e4)); - - Assert.Equal(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e2)); - Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e3)); - Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e4)); - } - - [ConditionalFact] - public void Sorted_collection_constant_expressions_with_different_insertion_order_are_equal() - { - var expressionComparer = ExpressionEqualityComparer.Instance; - - // Test sorted collections with elements in different order - var e1 = Constant(new SortedSet { "value2", "value1", "value3" }); - var e2 = Constant(new SortedSet { "value1", "value3", "value2" }); - var e3 = Constant(new SortedSet { "value1", "value2", "value4" }); - - Assert.True(expressionComparer.Equals(e1, e2)); - Assert.False(expressionComparer.Equals(e1, e3)); - - Assert.Equal(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e2)); - Assert.NotEqual(expressionComparer.GetHashCode(e1), expressionComparer.GetHashCode(e3)); - } - [ConditionalFact] // #30697 public void Lambda_parameters_names_are_taken_into_account() { From edd875f4b5902f2df1ccb01c7e2b4649566cf098 Mon Sep 17 00:00:00 2001 From: Ladislav Bitto Date: Thu, 20 Nov 2025 12:40:10 -0800 Subject: [PATCH 3/3] updating comments --- src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs b/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs index c6ce2d2b44f..ff42ac5b095 100644 --- a/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs +++ b/src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs @@ -2710,7 +2710,8 @@ public static IQueryable IgnoreQueryFilters( Expression.Call( instance: null, method: IgnoreNamedQueryFiltersMethodInfo.MakeGenericMethod(typeof(TEntity)), - // converting the collection to an array if it isn't already one to ensure consistent caching + // converting the collection to an array if it isn't already one to ensure consistent caching. Fixes #37112. + // #37212 may be a possible future solution providing broader capabilities around parameterizing collections. arguments: [source.Expression, Expression.Constant(filterKeys is string[] ? filterKeys : filterKeys.ToArray())])) : source;