Skip to content

Commit be6decc

Browse files
authored
Merge pull request #1469 from json-api-dotnet/simplify-parametrized-constants-in-sql
Replace CreateTupleAccessExpressionForConstant with simpler implementation
2 parents 587d972 + 3f62b77 commit be6decc

File tree

6 files changed

+39
-39
lines changed

6 files changed

+39
-39
lines changed

Diff for: src/JsonApiDotNetCore/Queries/QueryableBuilding/SelectClauseBuilder.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ private Expression CreateLambdaBodyInitializerForTypeHierarchy(FieldSelection se
9393
private static BinaryExpression CreateRuntimeTypeCheck(LambdaScope lambdaScope, Type concreteClrType)
9494
{
9595
// Emitting "resource.GetType() == typeof(Article)" instead of "resource is Article" so we don't need to check for most-derived
96-
// types first. This way, we can fallback to "anything else" at the end without worrying about order.
96+
// types first. This way, we can fall back to "anything else" at the end without worrying about order.
9797

98-
Expression concreteTypeConstant = concreteClrType.CreateTupleAccessExpressionForConstant(typeof(Type));
98+
Expression concreteTypeConstant = SystemExpressionBuilder.CloseOver(concreteClrType);
9999
MethodCallExpression getTypeCall = Expression.Call(lambdaScope.Accessor, TypeGetTypeMethod);
100100

101101
return Expression.MakeBinary(ExpressionType.Equal, getTypeCall, concreteTypeConstant, false, TypeOpEqualityMethod);

Diff for: src/JsonApiDotNetCore/Queries/QueryableBuilding/SkipTakeClauseBuilder.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public override Expression VisitPagination(PaginationExpression expression, Quer
3636

3737
private static Expression ExtensionMethodCall(Expression source, string operationName, int value, QueryClauseBuilderContext context)
3838
{
39-
Expression constant = value.CreateTupleAccessExpressionForConstant(typeof(int));
39+
Expression constant = SystemExpressionBuilder.CloseOver(value);
4040

4141
return Expression.Call(context.ExtensionType, operationName, [context.LambdaScope.Parameter.Type], source, constant);
4242
}

Diff for: src/JsonApiDotNetCore/Queries/QueryableBuilding/WhereClauseBuilder.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ public override Expression VisitNullConstant(NullConstantExpression expression,
245245

246246
public override Expression VisitLiteralConstant(LiteralConstantExpression expression, QueryClauseBuilderContext context)
247247
{
248-
Type type = expression.TypedValue.GetType();
249-
return expression.TypedValue.CreateTupleAccessExpressionForConstant(type);
248+
return SystemExpressionBuilder.CloseOver(expression.TypedValue);
250249
}
251250
}
+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
using System.Linq.Expressions;
2+
using System.Reflection;
3+
4+
#pragma warning disable AV1008
5+
6+
namespace JsonApiDotNetCore.Queries;
7+
8+
internal static class SystemExpressionBuilder
9+
{
10+
private static readonly MethodInfo CloseOverOpenMethod =
11+
typeof(SystemExpressionBuilder).GetMethods().Single(method => method is { Name: nameof(CloseOver), IsGenericMethod: true });
12+
13+
// To enable efficient query plan caching, inline constants (that vary per request) should be converted into query parameters.
14+
// https://stackoverflow.com/questions/54075758/building-a-parameterized-entityframework-core-expression
15+
//
16+
// CloseOver can be used to change a query like:
17+
// SELECT ... FROM ... WHERE x."Age" = 3
18+
// into:
19+
// SELECT ... FROM ... WHERE x."Age" = @p0
20+
21+
public static Expression CloseOver(object value)
22+
{
23+
ArgumentGuard.NotNull(value);
24+
25+
MethodInfo closeOverClosedMethod = CloseOverOpenMethod.MakeGenericMethod(value.GetType());
26+
return (Expression)closeOverClosedMethod.Invoke(null, [value])!;
27+
}
28+
29+
public static Expression CloseOver<T>(T value)
30+
{
31+
// From https://github.com/dotnet/efcore/issues/28151#issuecomment-1374480257.
32+
return ((Expression<Func<T>>)(() => value)).Body;
33+
}
34+
}

Diff for: src/JsonApiDotNetCore/Queries/SystemExpressionExtensions.cs

-33
This file was deleted.

Diff for: src/JsonApiDotNetCore/Resources/ResourceFactory.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public NewExpression CreateNewExpression(Type resourceClrType)
102102
{
103103
object constructorArgument = ActivatorUtilities.GetServiceOrCreateInstance(_serviceProvider, constructorParameter.ParameterType);
104104

105-
Expression argumentExpression = constructorArgument.CreateTupleAccessExpressionForConstant(constructorArgument.GetType());
105+
Expression argumentExpression = SystemExpressionBuilder.CloseOver(constructorArgument);
106106
constructorArguments.Add(argumentExpression);
107107
}
108108
#pragma warning disable AV1210 // Catch a specific exception instead of Exception, SystemException or ApplicationException

0 commit comments

Comments
 (0)