parameterize for perf

Author: SimonCropp

src/Directory.Build.props

@@ -2,7 +2,7 @@
 <Project>
   <PropertyGroup>
     <NoWarn>CS1591;NU5104;CS1573;CS9107;NU1608;NU1109</NoWarn>
-    <Version>34.2.0</Version>
+    <Version>34.2.2</Version>
     <LangVersion>preview</LangVersion>
     <AssemblyVersion>1.0.0</AssemblyVersion>
     <PackageTags>EntityFrameworkCore, EntityFramework, GraphQL</PackageTags>

src/GraphQL.EntityFramework/GlobalUsings.cs

@@ -4,4 +4,5 @@
 global using Microsoft.EntityFrameworkCore;
 global using Microsoft.EntityFrameworkCore.Metadata;
 global using Microsoft.EntityFrameworkCore.Query;
-global using Microsoft.Extensions.DependencyInjection;
+global using Microsoft.Extensions.DependencyInjection;
+global using System.Runtime.CompilerServices;

src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs

@@ -19,7 +19,7 @@ static class SelectExpressionBuilder
 
     static ConcurrentDictionary<Type, EntityTypeMetadata> entityMetadataCache = new();
 
-    record PropertyMetadata(PropertyInfo Property, bool CanWrite, MemberExpression PropertyAccess, MemberBinding? Binding, MethodInfo OrderByMethod);
+    record PropertyMetadata(PropertyInfo Property, bool CanWrite, bool IsAutoProperty, MemberExpression PropertyAccess, MemberBinding? Binding, MethodInfo OrderByMethod);
 
     record EntityTypeMetadata(
         ParameterExpression Parameter,
@@ -67,9 +67,15 @@ public static bool TryBuild<TEntity>(
             foreach (var keyName in projection.KeyNames)
             {
                 if (properties.TryGetValue(keyName, out var metadata) &&
-                    metadata.CanWrite &&
                     addedProperties.Add(keyName))
                 {
+                    if (!metadata.CanWrite || !metadata.IsAutoProperty)
+                    {
+                        // Key property has a custom setter that may throw during materialization,
+                        // or is read-only. Fall back to full entity loading so EF can use backing fields.
+                        return null;
+                    }
+
                     bindings.Add(metadata.Binding!);
                 }
             }
@@ -269,9 +275,16 @@ static bool TryBuildNavigationBindings(
             foreach (var keyName in projection.KeyNames)
             {
                 if (properties.TryGetValue(keyName, out var metadata) &&
-                    metadata.CanWrite &&
                     addedProperties.Add(keyName))
                 {
+                    if (!metadata.CanWrite || !metadata.IsAutoProperty)
+                    {
+                        // Key property has a custom setter that may throw during materialization,
+                        // or is read-only. Can't use projection.
+                        bindings = null;
+                        return false;
+                    }
+
                     bindings.Add(Expression.Bind(metadata.Property, Expression.Property(sourceExpression, metadata.Property)));
                 }
             }
@@ -435,9 +448,12 @@ static EntityTypeMetadata GetEntityMetadata(Type type) =>
             foreach (var property in properties)
             {
                 var propertyAccess = Expression.Property(parameter, property);
-                var binding = property.CanWrite ? Expression.Bind(property, propertyAccess) : null;
+                var canWrite = property.CanWrite;
+                var isAutoProperty = canWrite &&
+                                     property.SetMethod!.IsDefined(typeof(CompilerGeneratedAttribute), false);
+                var binding = canWrite ? Expression.Bind(property, propertyAccess) : null;
                 var orderByMethod = SelectExpressionBuilder.orderByMethod.MakeGenericMethod(type, property.PropertyType);
-                dictionary[property.Name] = new(property, property.CanWrite, propertyAccess, binding, orderByMethod);
+                dictionary[property.Name] = new(property, canWrite, isAutoProperty, propertyAccess, binding, orderByMethod);
             }
 
             var newInstance = Expression.New(type);

src/GraphQL.EntityFramework/Where/ExpressionBuilder.cs

@@ -228,11 +228,8 @@ static Expression GetExpression(string path, Comparison comparison, string?[]? v
 
     static MethodCallExpression MakeObjectListInComparision(string[] values, Property<T> property)
     {
-        // Attempt to convert the string values to the object type
         var objects = TypeConverter.ConvertStringsToList(values, property.Info);
-        // Make the object values a constant expression
-        var constant = Expression.Constant(objects);
-        // Build and return the expression body
+        var constant = MakeParameterizedConstant(objects, objects.GetType());
         return Expression.Call(constant, property.SafeListContains, property.Left);
     }
 
@@ -244,14 +241,14 @@ static MethodCallExpression MakeStringListInComparison(string[] values, Property
         var itemEvaluate = Expression.Lambda<Func<string, bool>>(equalsBody, ExpressionCache.StringParam);
 
         // Build Expression body to check if any string values match the property value
-        return Expression.Call(null, ReflectionCache.StringAny, Expression.Constant(values), itemEvaluate);
+        return Expression.Call(null, ReflectionCache.StringAny, MakeParameterizedConstant(values, typeof(string[])), itemEvaluate);
     }
 
     static Expression MakeSingleStringComparison(Comparison comparison, string? value, Property<T> property)
     {
         var left = property.Left;
 
-        var valueConstant = Expression.Constant(value, typeof(string));
+        var valueConstant = MakeParameterizedConstant(value, typeof(string));
         var nullCheck = Expression.NotEqual(left, ExpressionCache.Null);
 
         switch (comparison)
@@ -280,7 +277,7 @@ static Expression MakeSingleStringComparison(Comparison comparison, string? valu
     static Expression MakeSingleObjectComparison(Comparison comparison, object? value, Property<T> property)
     {
         var left = property.Left;
-        var constant = Expression.Constant(value, left.Type);
+        var constant = MakeParameterizedConstant(value, left.Type);
 
         return comparison switch
         {
@@ -294,6 +291,15 @@ static Expression MakeSingleObjectComparison(Comparison comparison, object? valu
         };
     }
 
+    // Wraps a value in a field access on a holder object so that EF Core
+    // emits a SQL parameter (@p0) instead of inlining the value as a literal.
+    static Expression MakeParameterizedConstant(object? value, Type targetType)
+    {
+        var holder = new ValueHolder(value);
+        Expression access = Expression.Field(Expression.Constant(holder), "Value");
+        return Expression.Convert(access, targetType);
+    }
+
     static bool HasListPropertyInPath(string path) =>
         path.Contains('[');
 
@@ -310,4 +316,9 @@ static Expression NegateExpression(Expression expression) =>
 
     [GeneratedRegex(@"\[(.*)\]")]
     private static partial Regex ListPropertyRegex();
+}
+
+class ValueHolder(object? value)
+{
+    public object? Value = value;
 }

src/SampleWeb.Tests/GraphQlControllerTests.Single_not_found.verified.txt

@@ -3,11 +3,15 @@
   Message:
 Not found.
 Query:
+DECLARE @p1 int = 99;
+
 SELECT [c].[Id]
 FROM [Companies] AS [c]
-WHERE [c].[Id] = 99,
+WHERE [c].[Id] = @p1,
   Query:
+DECLARE @p1 int = 99;
+
 SELECT [c].[Id]
 FROM [Companies] AS [c]
-WHERE [c].[Id] = 99
+WHERE [c].[Id] = @p1
 }

src/SampleWeb.Tests/SchemaPrint.cs

@@ -1,5 +1,4 @@
 using GraphQL;
-using GraphQL.Types;
 
 public class SchemaPrint
 {

src/Tests/IntegrationTests/Graphs/GuardedKeyEntity.cs

@@ -0,0 +1,30 @@
+public class GuardedKeyEntity
+{
+    Guid id;
+
+    public Guid Id
+    {
+        get => id;
+        set => throw new InvalidOperationException("Id is generated from initial EmailAddress");
+    }
+
+    public string? EmailAddress { get; set; }
+
+    public GuardedKeyEntity(string emailAddress)
+    {
+        id = CreateDeterministicGuid(emailAddress);
+        EmailAddress = emailAddress;
+    }
+
+    // EF Core needs a parameterless constructor; it will set the backing field directly
+    GuardedKeyEntity()
+    {
+    }
+
+    static Guid CreateDeterministicGuid(string input)
+    {
+        var bytes = Encoding.UTF8.GetBytes(input);
+        var hash = System.Security.Cryptography.SHA256.HashData(bytes);
+        return new(hash.AsSpan(0, 16));
+    }
+}

src/Tests/IntegrationTests/Graphs/GuardedKeyEntityGraphType.cs

@@ -0,0 +1,7 @@
+public class GuardedKeyEntityGraphType :
+    EfObjectGraphType<IntegrationDbContext, GuardedKeyEntity>
+{
+    public GuardedKeyEntityGraphType(IEfGraphQLService<IntegrationDbContext> graphQlService) :
+        base(graphQlService) =>
+        AutoMap();
+}

src/Tests/IntegrationTests/IntegrationDbContext.cs

@@ -54,6 +54,7 @@ protected override void OnConfiguring(DbContextOptionsBuilder builder) =>
     public DbSet<TphDerivedNavBaseEntity> TphDerivedNavBaseEntities { get; set; } = null!;
     public DbSet<CategoryEntity> CategoryEntities { get; set; } = null!;
     public DbSet<RegionEntity> RegionEntities { get; set; } = null!;
+    public DbSet<GuardedKeyEntity> GuardedKeyEntities { get; set; } = null!;
 
     protected override void OnModelCreating(ModelBuilder modelBuilder)
     {
@@ -173,6 +174,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             .HasOne(_ => _.Region)
             .WithMany()
             .HasForeignKey(_ => _.RegionId);
+        modelBuilder.Entity<GuardedKeyEntity>(entity =>
+        {
+            entity.Property(_ => _.Id).HasField("id");
+            entity.OrderBy(_ => _.EmailAddress);
+        });
         modelBuilder.Entity<CategoryEntity>()
             .OrderBy(_ => _.Name);
         modelBuilder.Entity<RegionEntity>()

src/Tests/IntegrationTests/IntegrationTests.AddSingleField_with_filter_accessing_navigation_denies_when_no_match.verified.txt

@@ -15,6 +15,9 @@ from   FilterReferenceEntities as f
        inner join
        FilterBaseEntities as f0
        on f.BaseEntityId = f0.Id
-where  f.Id = 'Guid_1'
+where  f.Id = @p1,
+    Parameters: {
+      @p1: Guid_1
+    }
   }
 }