Conversation
The case-sensitive @=, _=, _-= and their negations called the string method on the property without a null check. In memory, a null property threw NullReferenceException. The case-insensitive forms already checked for null. Add the same check: a null value does not contain, start with, or end with a value. BREAKING CHANGE: the expression of a case-sensitive string operator now has a null check, so the expression text changes. In memory, a null property no longer throws: @=, _=, and _-= skip it, and the negations include it.
pdevito3
force-pushed
the
fm/qk-breaking-string-null-guard
branch
from
October 1, 2026 21:47
1df8bf4 to
2a68372
Compare
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.
For later consideration in a major version. Do not merge now. #134 restored the v1.14.2 expression to keep v1.x compatible (restore commit
0abd1f8). This PR re-appliesaff9638. The captain decides on this PR separately.Summary
The case-sensitive string operators get a null check on the property, like their case-insensitive forms:
@=x.Title.Contains(v)(x.Title != null) AndAlso x.Title.Contains(v)_=x.Title.StartsWith(v)(x.Title != null) AndAlso x.Title.StartsWith(v)_-=x.Title.EndsWith(v)(x.Title != null) AndAlso x.Title.EndsWith(v)!@=Not(x.Title.Contains(v))(x.Title == null) OrElse Not(x.Title.Contains(v))!_=Not(x.Title.StartsWith(v))(x.Title == null) OrElse Not(x.Title.StartsWith(v))!_-=Not(x.Title.EndsWith(v))(x.Title == null) OrElse Not(x.Title.EndsWith(v))The change is in
QueryKit/Operators/ComparisonOperator.csonly.v1.14.2 behavior (and main)
The case-sensitive operators call the string method on the property with no null check. On
IEnumerableand on LINQ to Objects, a null property throwsNullReferenceException. The case-insensitive forms (@=*,_=*,_-=*and their negations) already check for null.New behavior
A null value does not contain, start with, or end with a value.
@=,_=, and_-=skip a null property.!@=,!_=, and!_-=include a null property. In memory, no operator throws on a null property.Example
NullReferenceException.lamb. The second call givesnullandother.Expression text for
Title _= "lam":x => x.Title.StartsWith("lam").x => ((x.Title != null) AndAlso x.Title.StartsWith("lam")).Justification
An in-memory filter must not crash on data that has a null string. The case-sensitive and case-insensitive forms of one operator must agree on null. The in-memory result now matches the Postgres (EF Core) result, which already includes a null property for the negated operators.
Migration
@=,_=, and_-=, the result rows do not change. For the negated operators, the rows can change on a provider that does not already include null. On Postgres (EF Core), the result rows do not change. The integration test below shows this with and without the null check.NullReferenceExceptionnow returns rows. Code that caught this exception must use the result instead.ToDisplayString()orToString()) sees the added null check.README
No change. The README does not document null handling for these operators.
Tests
These tests come back from main before #134:
FilterParserTests: 8 expectations in 8 tests have the null check again:complex_with_lots_of_types,starts_with_operator,ends_with_operator,multiple_properties_and_operators,complex_filter_with_nested_parentheses,ends_with_works,contains_is_case_sensitive,not_contains_works.OperatorAliasTests.can_use_contains_not_case_sensitive: 1 expectation has the null check again.FilterParsingRegressionTests:case_sensitive_string_operator_on_null_property_throws_in_memorybecomescase_sensitive_string_operator_handles_null_property(6 cases). The positive operators givelamb. The negated operators givenullandother.The integration test
case_sensitive_string_operator_handles_null_property(Postgres) does not change. It passes with and without the null check.dotnet test: 470 unit tests and 297 Postgres integration tests (Testcontainers) pass, 0 failures.Rebase on main
This branch is rebased on current main (#169). The rebase had no conflicts. The breaking change did not change.