Skip to content

fix(filter): filter a query name with HasConversion where v1.14.2 threw - #169

Merged
pdevito3 merged 1 commit into
mainfrom
fm/qk-153-split
Oct 1, 2026
Merged

pdevito3 merged 1 commit into
mainfrom
fm/qk-153-split

Conversation

@pdevito3

@pdevito3 pdevito3 commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

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

ParseFilter replaces a query name with the property path before the parser runs. The HasConversion lookup 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:

  • If the lookup by query name misses, and QueryKit cannot read a value of the property type, the parser finds the conversion by the property path. On v1.14.2, every input on this route threw.
  • A converted nullable struct is built from its underlying type, then converted. On v1.14.2, this comparison threw. A null literal keeps the v1.14.2 result.
  • A Guid string operator (for example @=) with HasConversion<string>() builds the right side for the string, not for the Guid.
  • A property list passes the resolved member path, so a property in another case finds its conversion.

Example (issue #106)

var config = new QueryKitConfiguration(config =>
{
    config.Property<TestingPerson>(x => x.Id).HasQueryName("wrappedid").HasConversion<string>();
});
rows.ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList();
  • v1.14.2 and main: ParsingException ("Unsupported value '2' for type 'WrappedId'").
  • This PR: the row with Id 2.

Fixed inputs (all threw on v1.14.2)

  • wrappedid == "2" and wrappedid != "2" on a converted struct with a query name, in either order of HasQueryName and HasConversion.
  • Id == "2" on the same property, by its property name.
  • wrappedid == "2" on a converted WrappedId?, with or without a query name.
  • mail == "a@x.com" and mail == null on a converted reference type with a query name.
  • Email == "a@x.com" by the property path when the query name is mail.
  • 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 == null on a converted reference type without a query name. v1.14.2 builds new 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 converted Email with a query name. v1.14.2 compares the child x.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 null Email, and EF cannot translate it.
  • A converted int, int?, enum, or Guid with a query name. v1.14.2 compares these by type (for example x.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.
  • Text that is not a Guid on a converted Guid with a query name and == (for example qn == "ab7afb17"). fix(filter)!: find HasConversion by the property path #153 builds new 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.

I also used the repo verify-querykit harness on a live Postgres 17 container. Id @= "000000000003" with HasConversion<string>() on Recipe.Id threw ArgumentException on main on both targets. With this PR, both targets return Salt Bread, and the SQL has WHERE 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_throws becomes can_filter_struct_with_query_name_and_has_conversion.
  • struct_with_has_conversion_configured_before_query_name_throws becomes can_filter_struct_with_has_conversion_configured_before_query_name.
  • struct_with_not_equals_query_name_and_has_conversion_throws becomes can_filter_struct_with_not_equals_query_name_and_has_conversion.
  • property_path_with_query_name_and_has_conversion_configured_throws becomes can_filter_by_property_path_when_query_name_and_has_conversion_are_configured.
  • reference_type_with_query_name_and_has_conversion_throws becomes can_filter_reference_type_with_query_name_and_has_conversion.
  • nested_property_with_query_name_and_has_conversion_throws becomes can_filter_nested_property_with_query_name_and_has_conversion.
  • nullable_struct_with_has_conversion_throws becomes can_filter_nullable_struct_with_has_conversion.
  • null_on_reference_type_with_query_name_and_has_conversion_throws becomes can_filter_null_on_reference_type_with_query_name_and_has_conversion.
  • guid_with_contains_and_has_conversion_throws becomes can_filter_guid_with_contains_and_has_conversion.

New unit tests:

  • can_filter_nullable_struct_with_query_name_and_has_conversion
  • can_filter_property_list_with_lowercase_path_query_name_and_has_conversion
  • Pin: int_with_query_name_and_has_conversion_compares_the_int
  • Pin: guid_with_query_name_and_has_conversion_compares_a_guid_constant

Unchanged pins from main: child_property_of_converted_parent_with_query_name_compares_the_child and null_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_conversion
  • can_filter_by_email_property_path_when_query_name_and_has_conversion_are_configured
  • can_filter_by_null_email_with_query_name_and_has_conversion
  • can_filter_by_nested_postal_code_with_query_name_and_has_conversion
  • can_filter_guid_with_contains_and_has_conversion
  • Pin: null_email_with_has_conversion_matches_no_row
  • Pin: email_value_with_query_name_and_has_conversion_compares_the_child

With the FilterParser.cs of main, the 11 unit fix tests and the 5 integration fix tests fail, and all the pins pass. With this PR, dotnet build succeeds, and dotnet test passes 470 unit tests and 297 integration tests with 0 failures.

The README does not change, because no documented behavior changes.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HasQueryName alias silently drops HasConversion for value-type (struct) properties

1 participant