Skip to content

fix(filter): throw UnknownFilterPropertyException again for a failed query name filter - #160

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

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

Conversation

@pdevito3

@pdevito3 pdevito3 commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Summary

This PR restores D5 from the v1.14.2 review report. It is not a breaking change.

A derived property or custom operation can have a query name with a space or a hyphen, for example double rating or full-name. v1.14.2 read only the first word (double) as a property and threw UnknownFilterPropertyException for it. Main reads the whole query name. When the filter then fails, main throws ParsingException or another exception, not UnknownFilterPropertyException.

This PR keeps the main behavior for a filter that succeeds. If a filter reads such a query name and then fails, the parser parses the filter again without these query names. The second parse reads the input like v1.14.2. Thus it throws the exception that v1.14.2 gave:

  • UnknownFilterPropertyException for the first word, when the query name comes first.
  • The earlier exception, when an earlier clause fails. For example Title == && is adult == true still throws ParsingException, and Age > "x" && is adult == true still throws FormatException.

The second parse runs only for a failed filter that read such a query name. Other filters run one parse, like before.

D1 (#155) removes the indexer case of the report. This PR covers the remaining cases.

Results against v1.14.2

double rating is a derived property, and rated over is a custom operation.

Input v1.14.2 main This PR
double rating > 6 UnknownFilterPropertyException('double') ParsingException same as v1.14.2
double rating UnknownFilterPropertyException('double') ParsingException same as v1.14.2
rated over UnknownFilterPropertyException('rated') ParsingException same as v1.14.2
rated over > 3 && Title == UnknownFilterPropertyException('rated') ParsingException same as v1.14.2
Rating > "x" && rated over > 3 FormatException FormatException same as v1.14.2
rated over > 3 UnknownFilterPropertyException('rated') filters filters, like main

The last row is not changed back. v1.14.2 rejected this input and main accepts it, so it is not a breaking change.

A differential probe ran 39 cases against v1.14.2 and this branch. The cases put the query name before and after a failed clause, inside groups, and with AllowUnknownProperties. The probe compared the expression, the in-memory rows, and the Postgres SQL. The only remaining differences are inputs that v1.14.2 rejected and main accepts, and message text that #157 changed.

Tests

  • Unit pins in the new file QueryKit.UnitTests/QueryNameOverUnknownTests.cs: 9 failed filters with a query name throw UnknownFilterPropertyException for the first word. 4 guard tests make sure that an earlier failure keeps its exception and that a filter with a query name still succeeds. The 9 pins fail on main.
  • A Postgres pin in the new file QueryKit.IntegrationTests/Tests/QueryNameOverUnknownTests.cs: a failed filter with the custom operation is adult throws UnknownFilterPropertyException, and the same query name filters the rows when the filter is correct. It fails on main.
  • dotnet test: unit 423 passed, integration 284 passed.

Overlap with other PRs

…query name filter

v1.14.2 read only the first word of a derived property or custom operation query name with a space or a hyphen, and threw UnknownFilterPropertyException for that word. Main reads the whole query name. When that filter then fails, main throws ParsingException.

If a filter that reads such a query name fails, the parser now parses it again without these query names. Thus it throws the exception that v1.14.2 gave. A filter that succeeds does not change.
@pdevito3
pdevito3 merged commit 94beca6 into main Oct 1, 2026
2 checks passed
@pdevito3
pdevito3 deleted the fm/qk-restore-d1-d5-d5 branch October 1, 2026 20:41
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