-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keep actual parameter values out of CommandCacheKey in RelationalCommandCache #34631
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,11 +106,16 @@ | |
} | ||
} | ||
|
||
private readonly struct CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues) | ||
private readonly struct CommandCacheKey | ||
: IEquatable<CommandCacheKey> | ||
{ | ||
private readonly Expression _queryExpression = queryExpression; | ||
private readonly IReadOnlyDictionary<string, object?> _parameterValues = parameterValues; | ||
private readonly Expression _queryExpression; | ||
private readonly IReadOnlyDictionary<string, ParameterValueInfo> _parameterValues; | ||
|
||
public CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues) { | ||
_queryExpression = queryExpression; | ||
_parameterValues = parameterValues.ToDictionary(p => p.Key, p => new ParameterValueInfo(p.Value)); | ||
} | ||
|
||
public override bool Equals(object? obj) | ||
=> obj is CommandCacheKey commandCacheKey | ||
|
@@ -133,18 +138,8 @@ | |
return false; | ||
} | ||
|
||
// ReSharper disable once ArrangeRedundantParentheses | ||
if ((value == null) != (otherValue == null)) | ||
{ | ||
if (value != otherValue) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep old behaviour, where we return true on first object[], or compare all values? |
||
return false; | ||
} | ||
|
||
if (value is IEnumerable | ||
&& value.GetType() == typeof(object[])) | ||
{ | ||
// FromSql parameters must have the same number of elements | ||
return ((object[])value).Length == (otherValue as object[])?.Length; | ||
} | ||
} | ||
} | ||
|
||
|
@@ -153,5 +148,16 @@ | |
|
||
public override int GetHashCode() | ||
=> RuntimeHelpers.GetHashCode(_queryExpression); | ||
|
||
private record ParameterValueInfo { | ||
public bool IsNull { get; init; } | ||
public int? ObjectArrayLength { get; init; } // FromSql parameters must have the same number of elements | ||
|
||
public ParameterValueInfo(object? parameterValue) { | ||
IsNull = parameterValue == null; | ||
var isObjectArray = parameterValue is IEnumerable && parameterValue.GetType() == typeof(object[]); | ||
ObjectArrayLength = isObjectArray ? ((object[])parameterValue).Length : null; | ||
Check failure on line 159 in src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs Azure Pipelines / efcore-ci (Build macOS)src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs#L159
Check failure on line 159 in src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs Azure Pipelines / efcore-ci (Build macOS)src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs#L159
Check failure on line 159 in src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs Azure Pipelines / efcore-ci (Build Linux)src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs#L159
Check failure on line 159 in src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs Azure Pipelines / efcore-ci (Build Linux)src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs#L159
|
||
} | ||
}; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameterValues contains all user data objects in queries before AsEnumerable or ToList, handle as radioactive. We only need meta data anyway