Skip to content

fix(filter): find the prevent-filter setting by the property path again - #156

Merged
pdevito3 merged 1 commit into
mainfrom
fm/qk-restore-d1-d5-d2
Oct 1, 2026
Merged

pdevito3 merged 1 commit into
mainfrom
fm/qk-restore-d1-d5-d2

Conversation

@pdevito3

@pdevito3 pdevito3 commented Oct 1, 2026

Copy link
Copy Markdown
Owner

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 PreventFilter setting 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: Serving has the query name directions. Directions has the query name notes and PreventFilter().
Config B: Title has the query name directions and PreventFilter(). Directions has the query name notes.

Config Input v1.14.2 main This PR
A notes == "Whisk and fry" True == True, no WHERE x.Directions == "Whisk and fry" (the prevented property filters) same as v1.14.2
A notes == "Whisk and fry" || Title == "Beef Stew" (True == True) OrElse ... filters on Directions same as v1.14.2
B notes == "Whisk and fry" x.Directions == "Whisk and fry" True == True, no WHERE same as v1.14.2
B notes == "Whisk and fry" || Title == "Beef Stew" (x.Directions == ...) OrElse (True == True) (True == True) OrElse (True == True) same as v1.14.2

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 HasConversion lookup on the next lines has the same two-step shape. The report marks a change there as optional. A probe with a HasConversion<int>() and a HasConversion<string>() mapping in the same chain gave the same result as v1.14.2, so this PR does not change that line.

Tests

  • Unit pins in QueryKit.UnitTests/PropertyResolverTests.cs for both configs. Both fail on main.
  • Postgres pins in QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs for 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

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.
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.

1 participant