fix(filter): find the prevent-filter setting by the property path again - #156
Merged
Merged
Conversation
The PreventFilter check looked up the property path as a query name first. When another property had a query name equal to that path, the check used the settings of that other property. A prevented property then filtered, and a property that was not prevented became a True clause. Look up the settings by the property path only, like v1.14.2.
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.
Summary
This PR restores D2 from the v1.14.2 review report. It is not a breaking change.
Alias replacement turns each query name into its property path before the grammar runs. Main then looks up the
PreventFiltersetting in two steps: it reads the property path as a query name again, and only then finds the settings. If another property has a query name equal to that path (ignoring case), main uses the settings of the other property.v1.14.2 looked up the setting by the property path only, in one step. This PR does the same.
Results against v1.14.2
Config A:
Servinghas the query namedirections.Directionshas the query namenotesandPreventFilter().Config B:
Titlehas the query namedirectionsandPreventFilter().Directionshas the query namenotes.notes == "Whisk and fry"True == True, no WHEREx.Directions == "Whisk and fry"(the prevented property filters)notes == "Whisk and fry" || Title == "Beef Stew"(True == True) OrElse ...Directionsnotes == "Whisk and fry"x.Directions == "Whisk and fry"True == True, no WHEREnotes == "Whisk and fry" || Title == "Beef Stew"(x.Directions == ...) OrElse (True == True)(True == True) OrElse (True == True)A differential probe ran 12 cases against v1.14.2 and this branch, on the expression, the in-memory rows, and the Postgres SQL. No difference remains.
The
HasConversionlookup on the next lines has the same two-step shape. The report marks a change there as optional. A probe with aHasConversion<int>()and aHasConversion<string>()mapping in the same chain gave the same result as v1.14.2, so this PR does not change that line.Tests
QueryKit.UnitTests/PropertyResolverTests.csfor both configs. Both fail on main.QueryKit.IntegrationTests/Tests/PropertyResolverTests.csfor both configs. Both fail on main. The first one also makes sure that the SQL has no WHERE clause.dotnet test: unit 404 passed, integration 285 passed.Overlap with other PRs
HasConversionlines right after the changed line. If fix(filter)!: find HasConversion by the property path #153 merges later, it can need a rebase.