fix(filter): filter a query name with HasConversion where v1.14.2 threw - #169
Merged
Merged
Conversation
A struct, a nullable struct, a reference type, or a nested property with a query name and HasConversion<string>() threw "Unsupported value". A filter by the property path and a property list also threw. The parser now finds the conversion by the property path when the lookup by query name misses, and only where v1.14.2 threw. A Guid with @= and HasConversion<string>() now compares the string form instead of throwing. Every input that v1.14.2 accepted gives the same result as before. Fixes #106
pdevito3
force-pushed
the
fm/qk-153-split
branch
from
October 1, 2026 21:16
c949292 to
41cc708
Compare
This was referenced Oct 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #106. Issue #106 is already closed, but this PR contains the fix for its example on v1.x.
#107 fixed the issue first. #134 removed that fix to keep v1.x compatible with v1.14.2. #153 holds the full fix as a breaking change. This PR is the non-breaking part of #153. It fixes only the inputs that threw on v1.14.2. #153 now keeps only the parts that change a v1.14.2 result.
Summary
ParseFilterreplaces a query name with the property path before the parser runs. TheHasConversionlookup then searches by query name, with the property path as the input. If the query name is different from the property name, the lookup finds nothing. QueryKit then cannot read the value and throws.This PR changes
QueryKit/FilterParser.cs:nullliteral keeps the v1.14.2 result.@=) withHasConversion<string>()builds the right side for the string, not for the Guid.Example (issue #106)
ParsingException("Unsupported value '2' for type 'WrappedId'").Id2.Fixed inputs (all threw on v1.14.2)
wrappedid == "2"andwrappedid != "2"on a converted struct with a query name, in either order ofHasQueryNameandHasConversion.Id == "2"on the same property, by its property name.wrappedid == "2"on a convertedWrappedId?, with or without a query name.mail == "a@x.com"andmail == nullon a converted reference type with a query name.Email == "a@x.com"by the property path when the query name ismail.zip == "..."on a nested converted property (PhysicalAddress.PostalCode).(id) == "2", a property list in another case, with a query name.Id @= "ab7afb17"on a converted Guid without a query name.Not in this PR (left in #153)
These parts change a result that v1.14.2 gave, or are part of the same route as such a change:
Email == nullon a converted reference type without a query name. v1.14.2 buildsnew EmailAddress("null")and the SQL is= 'null', so the filter matches no row. This PR keeps that result.Email.Value == "a@x.com"on a convertedEmailwith a query name. v1.14.2 compares the childx.Email.Value. Without a query name, v1.14.2 already compares the parent. This PR keeps both results. Thus the child filter with a query name still fails in memory for a nullEmail, and EF cannot translate it.int,int?, enum, or Guid with a query name. v1.14.2 compares these by type (for examplex.Num == 2, or a Guid constant). fix(filter)!: find HasConversion by the property path #153 sends them through the string conversion, so the expression changes or the filter throws. This PR keeps the v1.14.2 result.==(for exampleqn == "ab7afb17"). fix(filter)!: find HasConversion by the property path #153 buildsnew Guid("ab7afb17"), which still throws when the query runs. The same route changes an accepted Guid filter, so it stays in fix(filter)!: find HasConversion by the property path #153.Proof that this PR is not a break
I built one differential driver two times: against the QueryKit 1.14.2 NuGet package, and against the project source. The driver runs 4840 filters through the public API. It uses 11 property shapes (converted struct, nullable struct, reference type, nested reference type, Guid, Guid?, int, int?, enum, and two strings) and 7 configurations (none, conversion, query name, both in each order, an uppercase query name, and a lowercase query name). For each filter, it records three results: the expression text, the rows in memory, and the EF Core Npgsql SQL.
qn @= "2"on a converted struct now gives the sameArgumentExceptionasWid @= "2"without a query name.I also used the repo
verify-querykitharness on a live Postgres 17 container.Id @= "000000000003"withHasConversion<string>()onRecipe.IdthrewArgumentExceptionon main on both targets. With this PR, both targets returnSalt Bread, and the SQL hasWHERE r."Id"::text LIKE '%000000000003%'.Tests
Unit (
QueryKit.UnitTests/HasConversionTests.cs). The main tests that expected a fixed input to throw now expect rows:struct_with_query_name_and_has_conversion_throwsbecomescan_filter_struct_with_query_name_and_has_conversion.struct_with_has_conversion_configured_before_query_name_throwsbecomescan_filter_struct_with_has_conversion_configured_before_query_name.struct_with_not_equals_query_name_and_has_conversion_throwsbecomescan_filter_struct_with_not_equals_query_name_and_has_conversion.property_path_with_query_name_and_has_conversion_configured_throwsbecomescan_filter_by_property_path_when_query_name_and_has_conversion_are_configured.reference_type_with_query_name_and_has_conversion_throwsbecomescan_filter_reference_type_with_query_name_and_has_conversion.nested_property_with_query_name_and_has_conversion_throwsbecomescan_filter_nested_property_with_query_name_and_has_conversion.nullable_struct_with_has_conversion_throwsbecomescan_filter_nullable_struct_with_has_conversion.null_on_reference_type_with_query_name_and_has_conversion_throwsbecomescan_filter_null_on_reference_type_with_query_name_and_has_conversion.guid_with_contains_and_has_conversion_throwsbecomescan_filter_guid_with_contains_and_has_conversion.New unit tests:
can_filter_nullable_struct_with_query_name_and_has_conversioncan_filter_property_list_with_lowercase_path_query_name_and_has_conversionint_with_query_name_and_has_conversion_compares_the_intguid_with_query_name_and_has_conversion_compares_a_guid_constantUnchanged pins from main:
child_property_of_converted_parent_with_query_name_compares_the_childandnull_on_reference_type_with_has_conversion_matches_no_row.Postgres integration (
QueryKit.IntegrationTests/Tests/HasConversionTests.cs, Testcontainers):can_filter_by_email_with_query_name_and_has_conversioncan_filter_by_email_property_path_when_query_name_and_has_conversion_are_configuredcan_filter_by_null_email_with_query_name_and_has_conversioncan_filter_by_nested_postal_code_with_query_name_and_has_conversioncan_filter_guid_with_contains_and_has_conversionnull_email_with_has_conversion_matches_no_rowemail_value_with_query_name_and_has_conversion_compares_the_childWith the
FilterParser.csof main, the 11 unit fix tests and the 5 integration fix tests fail, and all the pins pass. With this PR,dotnet buildsucceeds, anddotnet testpasses 470 unit tests and 297 integration tests with 0 failures.The README does not change, because no documented behavior changes.