Skip to content

Keep actual parameter values out of CommandCacheKey in RelationalCommandCache #34631

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Keep actual parameter values out of CommandCacheKey in RelationalCommandCache #34631

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 20 additions & 14 deletions src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,16 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
}
}

private readonly struct CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
Copy link
Author

@cliffankh0z cliffankh0z Sep 9, 2024 •
edited
Loading

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

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
Expand All @@ -133,18 +138,8 @@ public bool Equals(CommandCacheKey commandCacheKey)
return false;
}

// ReSharper disable once ArrangeRedundantParentheses
if ((value == null) != (otherValue == null))
{
if (value != otherValue)
Copy link
Author

@cliffankh0z cliffankh0z Sep 11, 2024

Choose a reason for hiding this comment

The 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;
}
}
}

Expand All @@ -153,5 +148,16 @@ public bool Equals(CommandCacheKey commandCacheKey)

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;
}
};
}
}