Skip to content

Commit bf75e0b

Browse files
pdevito3claude
pdevito3
and
claude
committed
fix: use direct GUID comparison instead of ToString() for better EF Core translation
Previously, QueryKit converted all GUID comparisons to string using .ToString(), which caused EF Core translation failures in complex queries with multiple conditions. This fix distinguishes between operator types: - Equality/comparison operators (==, !=, <, >, in) now use direct GUID comparison - String operators (Contains, StartsWith, EndsWith) still use .ToString() as required Changes: - Add IsStringComparisonOperator() method to identify string-based operators - Update GUID handling in FilterParser to conditionally convert based on operator type - Fix GUID array handling for 'in' operator to maintain type consistency - Remove forced GUID-to-string conversion in InType operator - Update test expectations to reflect new behavior Benefits: - Fixes EF Core translation errors in complex GUID filter scenarios - Generates more efficient SQL (direct UUID comparison vs string cast) - Maintains backward compatibility for string operations on GUIDs - All 266 tests passing Resolves issue where filters like `id == "guid" && status == "value"` failed with "The LINQ expression could not be translated" error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e5c49fb commit bf75e0b

5 files changed

Lines changed: 170 additions & 24 deletions

File tree

‎QueryKit.IntegrationTests/Tests/GuidFilterBugTests.cs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
namespace QueryKit.IntegrationTests.Tests;
2+
3+
using Bogus;
4+
using Configuration;
5+
using FluentAssertions;
6+
using Microsoft.EntityFrameworkCore;
7+
using SharedTestingHelper.Fakes;
8+
using WebApiTestProject.Entities;
9+
using Xunit.Abstractions;
10+
11+
public class GuidFilterBugTests(ITestOutputHelper testOutputHelper) : TestBase
12+
{
13+
[Fact]
14+
public async Task guid_filter_with_converted_property_should_reproduce_ef_core_translation_error()
15+
{
16+
// Arrange
17+
var testingServiceScope = new TestingServiceScope();
18+
var faker = new Faker();
19+
20+
var targetId = Guid.Parse("ab7afb17-abca-4530-9f8f-bd1497e0f6be");
21+
var targetEmail = faker.Internet.Email();
22+
var targetPerson = new FakeTestingPersonBuilder()
23+
.WithId(targetId)
24+
.WithEmail(targetEmail)
25+
.WithFirstName("Target")
26+
.Build();
27+
28+
var otherSucceededEmail = faker.Internet.Email();
29+
var otherPerson = new FakeTestingPersonBuilder()
30+
.WithEmail(otherSucceededEmail)
31+
.WithFirstName("Other")
32+
.Build();
33+
34+
await testingServiceScope.InsertAsync(targetPerson, otherPerson);
35+
36+
// Configure QueryKit with custom query names like the user's real scenario
37+
// This mimics: Status (with EF HasConversion) and Id (Guid)
38+
var config = new QueryKitConfiguration(config =>
39+
{
40+
config.Property<TestingPerson>(x => x.Email)
41+
.HasQueryName("email")
42+
.HasConversion<string>();
43+
config.Property<TestingPerson>(x => x.Id)
44+
.HasQueryName("id");
45+
});
46+
47+
// Filter using both email (converted property) and id (Guid)
48+
var input = $"""email == "{targetEmail}" && id == "{targetId}" """;
49+
50+
// Act
51+
var queryablePeople = testingServiceScope.DbContext().People;
52+
var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config);
53+
54+
// This should throw: System.InvalidOperationException:
55+
// The LINQ expression 'DbSet<TestingPerson>()
56+
// .Where(d => d.Id.ToString() == "ab7afb17-abca-4530-9f8f-bd1497e0f6be" && ...)'
57+
// could not be translated
58+
var people = await appliedQueryable.ToListAsync();
59+
60+
// Assert
61+
people.Count.Should().Be(1);
62+
people[0].Id.Should().Be(targetId);
63+
people[0].Email.Value.Should().Be(targetEmail);
64+
people[0].FirstName.Should().Be("Target");
65+
}
66+
67+
[Fact]
68+
public async Task can_filter_by_guid_id_with_custom_query_name_and_additional_filters()
69+
{
70+
// Arrange
71+
var testingServiceScope = new TestingServiceScope();
72+
73+
var targetId = Guid.Parse("cd8efb28-bcdb-5641-af9f-ce2498f1c7cf");
74+
var targetPerson = new FakeTestingPersonBuilder()
75+
.WithId(targetId)
76+
.WithFirstName("Target")
77+
.WithLastName("Person")
78+
.Build();
79+
80+
var otherPerson = new FakeTestingPersonBuilder()
81+
.WithFirstName("Other")
82+
.WithLastName("Different")
83+
.Build();
84+
85+
await testingServiceScope.InsertAsync(targetPerson, otherPerson);
86+
87+
// Configure QueryKit with custom query name for Id property
88+
var config = new QueryKitConfiguration(config =>
89+
{
90+
config.Property<TestingPerson>(x => x.Id)
91+
.HasQueryName("id");
92+
});
93+
94+
// Filter with both id and lastname
95+
var input = $"""id == "{targetId}" && LastName == "Person" """;
96+
97+
// Act
98+
var queryablePeople = testingServiceScope.DbContext().People;
99+
var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config);
100+
101+
// This should throw the same EF Core translation error
102+
var people = await appliedQueryable.ToListAsync();
103+
104+
// Assert
105+
people.Count.Should().Be(1);
106+
people[0].Id.Should().Be(targetId);
107+
people[0].FirstName.Should().Be("Target");
108+
people[0].LastName.Should().Be("Person");
109+
}
110+
}

‎QueryKit.UnitTests/CustomFilterPropertyTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,9 @@ public void can_have_custom_prop_name_for_multiple_props()
119119
config.Property<TestingPerson>(x => x.Id).HasQueryName("identifier");
120120
});
121121
var filterExpression = FilterParser.ParseFilter<TestingPerson>(input, config);
122-
filterExpression.ToString().Should().Be($"""x => ((x.Title == "{stringValue}") OrElse (x.Id.ToString() == "{guidValue}"))""");
122+
filterExpression.ToString().Should().Be($"""x => ((x.Title == "{stringValue}") OrElse (x.Id == {guidValue}))""");
123123
}
124-
124+
125125
[Fact]
126126
public void can_have_custom_prop_name_for_some_props()
127127
{
@@ -135,7 +135,7 @@ public void can_have_custom_prop_name_for_some_props()
135135
config.Property<TestingPerson>(x => x.Title).HasQueryName("special_title");
136136
});
137137
var filterExpression = FilterParser.ParseFilter<TestingPerson>(input, config);
138-
filterExpression.ToString().Should().Be($"""x => ((x.Title == "{stringValue}") OrElse (x.Id.ToString() == "{guidValue}"))""");
138+
filterExpression.ToString().Should().Be($"""x => ((x.Title == "{stringValue}") OrElse (x.Id == {guidValue}))""");
139139
}
140140

141141
[Fact]

‎QueryKit.UnitTests/FilterParserTests.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public void complex_with_lots_of_types()
4242

4343
var filterExpression = FilterParser.ParseFilter<TestingPerson>(input);
4444
filterExpression.ToString().Should()
45-
.Be(""""x => (((((((x.Title.ToLower().Contains("waffle & chicken".ToLower()) AndAlso (x.Age > 30)) OrElse (x.Id.ToString() == "aa648248-cb69-4217-ac95-d7484795afb2")) OrElse (x.Title == "lamb")) OrElse (x.Title == null)) AndAlso ((x.Age < 18) OrElse ((x.BirthMonth == new Nullable`1(January)) AndAlso x.Title.StartsWith("ally")))) OrElse (x.Rating > 3.5)) OrElse ((x.SpecificDate == new Nullable`1(new DateTimeOffset(637922304030000000, 00:00:00))) AndAlso ((x.Date == new Nullable`1(new DateOnly(2022, 7, 1))) OrElse (x.Time == new Nullable`1(new TimeOnly(0, 0, 3, 0, 0))))))"""");
45+
.Be(""""x => (((((((x.Title.ToLower().Contains("waffle & chicken".ToLower()) AndAlso (x.Age > 30)) OrElse (x.Id == aa648248-cb69-4217-ac95-d7484795afb2)) OrElse (x.Title == "lamb")) OrElse (x.Title == null)) AndAlso ((x.Age < 18) OrElse ((x.BirthMonth == new Nullable`1(January)) AndAlso x.Title.StartsWith("ally")))) OrElse (x.Rating > 3.5)) OrElse ((x.SpecificDate == new Nullable`1(new DateTimeOffset(637922304030000000, 00:00:00))) AndAlso ((x.Date == new Nullable`1(new DateOnly(2022, 7, 1))) OrElse (x.Time == new Nullable`1(new TimeOnly(0, 0, 3, 0, 0))))))"""");
4646
}
4747

4848
[Fact]
@@ -76,7 +76,7 @@ public void can_handle_guid_with_double_quotes()
7676
var guid = Guid.NewGuid();
7777
var input = $"""Id == "{guid}" """;
7878
var filterExpression = FilterParser.ParseFilter<TestingPerson>(input);
79-
filterExpression.ToString().Should().Be($"x => (x.Id.ToString() == \"{guid}\")");
79+
filterExpression.ToString().Should().Be($"x => (x.Id == {guid})");
8080
}
8181

8282
[Fact]
@@ -85,15 +85,15 @@ public void can_handle_guid_without_double_quotes()
8585
var guid = Guid.NewGuid();
8686
var input = $"""Id == {guid} """;
8787
var filterExpression = FilterParser.ParseFilter<TestingPerson>(input);
88-
filterExpression.ToString().Should().Be($"x => (x.Id.ToString() == \"{guid}\")");
88+
filterExpression.ToString().Should().Be($"x => (x.Id == {guid})");
8989
}
9090

9191
[Fact]
9292
public void can_handle_guid_with_null()
9393
{
9494
var input = $"""SecondaryId == null """;
9595
var filterExpression = FilterParser.ParseFilter<Recipe>(input);
96-
filterExpression.ToString().Should().Be($"x => (IIF(x.SecondaryId.HasValue, x.SecondaryId.Value.ToString(), null) == null)");
96+
filterExpression.ToString().Should().Be($"x => (x.SecondaryId == null)");
9797
}
9898

9999
[Fact]
@@ -225,7 +225,7 @@ public void simple_in_operator_for_guid()
225225
var input = """Id ^^ ["6d623e92-d2cf-4496-a2df-f49fa77328ee"]""";
226226
var filterExpression = FilterParser.ParseFilter<TestingPerson>(input);
227227
filterExpression.ToString().Should()
228-
.Be(""""x => value(System.Collections.Generic.List`1[System.String]).Contains(x.Id.ToString())"""");
228+
.Be(""""x => value(System.Collections.Generic.List`1[System.Guid]).Contains(x.Id)"""");
229229
}
230230

231231
[Fact]

‎QueryKit/FilterParser.cs‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,12 @@ private static Expression CreateRightExprFromType(Type leftExprType, string righ
328328

329329
if (right.StartsWith("[") && right.EndsWith("]"))
330330
{
331-
targetType = targetType == typeof(Guid) || targetType == typeof(Guid?) ? typeof(string) : targetType;
331+
// Only convert GUID arrays to string arrays for string operators (Contains, etc.)
332+
// For other operators like 'in', keep as GUID array for proper type matching
333+
if ((targetType == typeof(Guid) || targetType == typeof(Guid?)) && op.IsStringComparisonOperator())
334+
{
335+
targetType = typeof(string);
336+
}
332337
var values = right.Trim('[', ']').Split(',').Select(x => x.Trim()).ToList();
333338
var elementType = targetType.IsArray ? targetType.GetElementType() : targetType;
334339

@@ -433,7 +438,16 @@ private static Expression CreateRightExprFromType(Type leftExprType, string righ
433438

434439
if (targetType == typeof(Guid))
435440
{
436-
return Expression.Constant(right, typeof(string));
441+
// For string operators (Contains, StartsWith, EndsWith), we need to compare as strings
442+
// For equality/comparison operators, we can compare GUIDs directly (more efficient and EF-friendly)
443+
if (op.IsStringComparisonOperator())
444+
{
445+
return Expression.Constant(right, typeof(string));
446+
}
447+
448+
// Parse the GUID for direct comparison
449+
var guidValue = Guid.Parse(right);
450+
return Expression.Constant(guidValue, typeof(Guid));
437451
}
438452

439453
var convertedValue = conversionFunction(right);
@@ -625,9 +639,18 @@ private static Parser<Expression> ComparisonExprParser<T>(ParameterExpression pa
625639
{
626640
guidPropertyPath = GetPropertyPath(guidMemberExpr, parameter);
627641
}
628-
629-
var guidStringExpr = HandleGuidConversion(temp.leftExpr, temp.leftExpr.Type);
630-
return temp.op.GetExpression<T>(guidStringExpr, CreateRightExpr(temp.leftExpr, temp.right, temp.op, config, guidPropertyPath),
642+
643+
// Only convert to string for operators that require string comparison (Contains, StartsWith, etc.)
644+
// For equality/comparison operators, keep as GUID for better EF Core translation
645+
if (temp.op.IsStringComparisonOperator())
646+
{
647+
var guidStringExpr = HandleGuidConversion(temp.leftExpr, temp.leftExpr.Type);
648+
return temp.op.GetExpression<T>(guidStringExpr, CreateRightExpr(temp.leftExpr, temp.right, temp.op, config, guidPropertyPath),
649+
config?.DbContextType);
650+
}
651+
652+
// For non-string operators, use direct GUID comparison
653+
return temp.op.GetExpression<T>(temp.leftExpr, CreateRightExpr(temp.leftExpr, temp.right, temp.op, config, guidPropertyPath),
631654
config?.DbContextType);
632655
}
633656

@@ -638,16 +661,20 @@ private static Parser<Expression> ComparisonExprParser<T>(ParameterExpression pa
638661
if (rightPropertyExpr != null)
639662
{
640663
// Handle GUID conversion for property-to-property comparisons
664+
// Only convert to string for string operators
641665
var leftExpr = temp.leftExpr;
642-
if (leftExpr.Type == typeof(Guid) || leftExpr.Type == typeof(Guid?))
643-
{
644-
leftExpr = HandleGuidConversion(leftExpr, leftExpr.Type);
645-
}
646-
if (rightPropertyExpr.Type == typeof(Guid) || rightPropertyExpr.Type == typeof(Guid?))
666+
if (temp.op.IsStringComparisonOperator())
647667
{
648-
rightPropertyExpr = HandleGuidConversion(rightPropertyExpr, rightPropertyExpr.Type);
668+
if (leftExpr.Type == typeof(Guid) || leftExpr.Type == typeof(Guid?))
669+
{
670+
leftExpr = HandleGuidConversion(leftExpr, leftExpr.Type);
671+
}
672+
if (rightPropertyExpr.Type == typeof(Guid) || rightPropertyExpr.Type == typeof(Guid?))
673+
{
674+
rightPropertyExpr = HandleGuidConversion(rightPropertyExpr, rightPropertyExpr.Type);
675+
}
649676
}
650-
677+
651678
// Ensure compatible types for property-to-property comparison
652679
var (leftCompatible, rightCompatible) = EnsureCompatibleTypes(leftExpr, rightPropertyExpr);
653680
return temp.op.GetExpression<T>(leftCompatible, rightCompatible, config?.DbContextType);

‎QueryKit/Operators/ComparisonOperator.cs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,17 @@ public static ComparisonOperator GetByOperatorString(string op, bool caseInsensi
178178
public bool CaseInsensitive { get; protected set; }
179179
public bool UsesAll { get; protected set; }
180180
public abstract Expression GetExpression<T>(Expression left, Expression right, Type? dbContextType);
181+
182+
/// <summary>
183+
/// Returns true if this operator requires string comparison (e.g., Contains, StartsWith, EndsWith).
184+
/// This is used to determine whether GUIDs should be converted to strings for comparison.
185+
/// </summary>
186+
public bool IsStringComparisonOperator()
187+
{
188+
return this is ContainsType or NotContainsType or StartsWithType or NotStartsWithType
189+
or EndsWithType or NotEndsWithType or SoundsLikeType or DoesNotSoundLikeType;
190+
}
191+
181192
protected ComparisonOperator(string name, int value, bool caseInsensitive = false, bool usesAll = false) : base(name, value)
182193
{
183194
CaseInsensitive = caseInsensitive;
@@ -515,10 +526,8 @@ public InType(bool caseInsensitive = false, bool usesAll = false) : base("^^", 1
515526
public override bool IsCountOperator() => false;
516527
public override Expression GetExpression<T>(Expression left, Expression right, Type? dbContextType)
517528
{
518-
var leftType = left.Type == typeof(Guid) || left.Type == typeof(Guid?)
519-
? typeof(string)
520-
: left.Type;
521-
529+
var leftType = left.Type;
530+
522531
if (right is NewArrayExpression newArrayExpression)
523532
{
524533
var listType = typeof(List<>).MakeGenericType(leftType);

0 commit comments

Comments
 (0)