Conversation
The list-member overload, Filter(type, x => x.Items, x => x.Member, value), walks the list with JSON_TREE and handed every node it yielded to JSON_EACH. JSON_TREE yields the scalar leaves as well as the element objects, and a string leaf -- any string member, or a DateTime, which both serializers write as text -- is not JSON, so JSON_EACH raised SQLite error 1 "malformed JSON". EXISTS stops at the first element that satisfies the predicate, so the error only surfaced for documents whose elements never match: the filter appeared to work until the store held a row it had to reject. The one existing test never set a string member, so it never reached a leaf. JSON_EACH now receives its input through a CASE that yields NULL for any node that is not an object, and JSON_EACH on NULL produces no rows. This is safe by SQL semantics rather than by planner placement: a JT.type conjunct in the WHERE clause is also coded in the outer loop today, but the CASE form does not depend on that. Arrays are excluded along with scalars, since a $.Member path cannot resolve on one, and JSON_TREE still visits nested lists of objects, so the depth searched is unchanged. Writing the NotEquals test surfaced a second defect on the same path: a DateTime value was rendered with ToString(), "1/1/2026 8:00:00 AM" under en-US, against a stored "2026-01-01T08:00:00Z", so no element ever compared equal and every document with a non-empty list matched. NotEquals now formats through DateTimeSerializationFormat as Equals already did. The top-level NotEquals branch has the same defect and is left for a separate change. The shape dates from the original inner-list commit and first shipped in v2.0.0. Tests: 318 passed, 4 pre-existing skips; 8 new.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
All reviewed changes are covered by tests and no blocking issues remain.
Review effort: Lite
Findings: None
What changed in this PR
Fixes list-member filters that could fail on scalar JSON leaves and corrects DateTime comparisons for NotEquals.
Changes:
- Restricts
JSON_EACHinputs to object nodes. - Uses serializer-formatted
DateTimevalues. - Adds regression tests and changelog entries.
| File | Description |
|---|---|
TychoDB/FilterBuilder.cs |
Fixes JSON traversal and DateTime inequality formatting. |
TychoDB.UnitTests/ListMemberFilterTests.cs |
Adds list-member filter regression coverage. |
CHANGELOG.md |
Documents both fixes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
The list-member overload, Filter(type, x => x.Items, x => x.Member, value),
walks the list with JSON_TREE and handed every node it yielded to JSON_EACH.
JSON_TREE yields the scalar leaves as well as the element objects, and a
string leaf -- any string member, or a DateTime, which both serializers write
as text -- is not JSON, so JSON_EACH raised SQLite error 1 "malformed JSON".
EXISTS stops at the first element that satisfies the predicate, so the error
only surfaced for documents whose elements never match: the filter appeared
to work until the store held a row it had to reject. The one existing test
never set a string member, so it never reached a leaf.
JSON_EACH now receives its input through a CASE that yields NULL for any node
that is not an object, and JSON_EACH on NULL produces no rows. This is safe by
SQL semantics rather than by planner placement: a JT.type conjunct in the
WHERE clause is also coded in the outer loop today, but the CASE form does not
depend on that. Arrays are excluded along with scalars, since a $.Member path
cannot resolve on one, and JSON_TREE still visits nested lists of objects, so
the depth searched is unchanged.
Writing the NotEquals test surfaced a second defect on the same path: a
DateTime value was rendered with ToString(), "1/1/2026 8:00:00 AM" under
en-US, against a stored "2026-01-01T08:00:00Z", so no element ever compared
equal and every document with a non-empty list matched. NotEquals now
formats through DateTimeSerializationFormat as Equals already did. The
top-level NotEquals branch has the same defect and is left for a separate
change.
The shape dates from the original inner-list commit and first shipped in
v2.0.0.
Tests: 318 passed, 4 pre-existing skips; 8 new.