fix(filter): accept a query name that is not a plain identifier - #115
Merged
Merged
Conversation
A query name can be any text, for example first-name, _first, or first name. v1.14.2 accepted it, because a regex pass replaced the query name before the parse. Main reads only letters, digits, and underscores as a property name, so first-name == "Ann" throws. The grammar now matches the configured query names before the general identifier, on the left side and in property lists. It tries the longest name first, ignores case, and needs a name boundary after the name. Values inside quotes stay untouched. Arithmetic still reads identifiers only, because a hyphen there is a minus sign.
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.
Why this is warranted
A query name can be any text, for example
first-name. v1.14.2 accepted it. Main reads only letters, digits, and underscores as a property name, sofirst-name == "Ann"throws. This PR makes the grammar match configured query names before the general identifier. Values inside quotes stay untouched.This is regression J in the breaking-change audit. It came in with #113 (
3c85253), which removed the regex pre-pass. This fix restores the v1.14.2 behavior and adds no breaking change.Summary
QueryKitPropertyMappings.QueryNames(internal) gives every query name of a property, a derived property, and a custom operation.verify-querykitharness has a newloose-namespreset for this proof.Evidence
Audit proof rows J1 to J8, as regression tests in
QueryKit.UnitTests/PropertyResolverTests.cs:first-name == "Ann"UnknownFilterPropertyException ... 'first'x => (x.FirstName == "Ann")_first == "Ann"ParsingExceptionx => (x.FirstName == "Ann")first name == "Ann"UnknownFilterPropertyException ... 'first'x => (x.FirstName == "Ann")person.first == "Ann"Title == "first-name == x"Title == first"first""first""first"first-name descfirst_name == "Ann"More tests: the case of the query name is ignored, the longer name wins (
first nameandfirst),firstdoes not match the start ofFirstName, a hyphen query name works in a property list, and a derived property can have a hyphen query name. The new integration test runs J1 to J3 on Postgres.dotnet testpasses: 359 unit tests and 271 integration tests, 0 failures.Live proof with the
verify-querykitharness,ApplyQueryKitFilteron a list and on EF Core with Postgres:Merge Danger
Door: two-way
The change is in the parser only. No public API or stored data changes. A revert restores main as it is now.
Blast Radius: filters
A configured query name now wins over the general identifier on the left side and in property lists. v1.14.2 also replaced a configured query name before the parse, so this order is the same as v1.14.2. In arithmetic, only identifier query names resolve, as on main. A query name with a
-in arithmetic is not supported, because(a-b)can mean a minus.