fix: bind the caller's filter as a single conjunct

[michaelstonis]

Stacked on #23. Review that one first; this diff is against it.

The headline fix, from an external bug report evaluating TychoDB 5.1.4 against a 257,974-row item master. It reproduced, and turned out to be worse than reported.

An ungrouped Or() escaped the partition and type predicates

FilterBuilder appended the caller's terms bare, so the generated clause read:

WHERE FullTypeName = ? AND Partition = ? AND <term1> OR <term2>

AND binds tighter than OR, so SQL parsed that as (FullTypeName = ? AND Partition = ? AND term1) OR (term2). Every term after the first Or() was matched against the whole table.

var f = FilterBuilder<ItemModel>.Create()
    .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or()
    .Filter(FilterType.Equals, x => x.DepartmentId, 47);

await db.ReadObjectsAsync<ItemModel>("partitionA", f);
// was: ids [1, 2, 4]  — id 4 lives in partitionB

With two types in one partition it returned a VendorModel deserialized as an ItemModel, no exception.

Worse than reported: the same clause backs DeleteObjectsAsync, so an ungrouped Or() could delete rows in other partitions and of other types (my repro deletes 3 rows where 2 were asked for), and CountObjectsAsync over-counted.

The caller's filter is now emitted inside its own parentheses — exactly the fix the reporter suggested:

... Where FullTypeName = $fullTypeName AND Partition = $partition AND ( CAST(...) = 33 OR CAST(...) = 47 )

Losing the Partition predicate also cost the partition-prefixed indexes, so this was a large performance regression too. A test asserts via EXPLAIN QUERY PLAN that a two-term OR-chain reaches the index again.

The same failure one level down, in LINQ

Found while investigating the above, and not in the original report. TychoQueryable emitted && and || operands flat, so Where(x => (x.A || x.B) && x.C) became A OR B AND C — read by SQL as A OR (B AND C) — returning rows that matched only A despite failing C. Wrong precedence within the predicate, independent of the partition leak. Each composite boolean node is now emitted in its own group.

Why it went unnoticed

Every regression test here seeds more than one partition and more than one type. A single-partition, single-type fixture passes even with the bug present, which is presumably why the existing suite was green.

Breaking change

An ungrouped Or() now means what it reads as. Code that unknowingly depended on the leaked rows returns fewer rows. That is the fix, not a regression.

Verification

237 tests pass; dotnet build TychoDB.sln -c Release clean.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes a critical SQL precedence bug where caller-supplied filters containing Or() could escape the mandatory FullTypeName + Partition predicates, causing cross-partition/type reads (and potentially deletes/counts) and preventing partition-prefixed indexes from being used. The fix groups the caller filter as a single conjunct in generated SQL, and additionally preserves boolean precedence when translating LINQ && / || expressions.

Changes:

• Wrap the caller’s FilterBuilder output in parentheses when appended to the base WHERE clause, preventing OR from splitting the partition/type predicates.
• Update LINQ predicate translation (TychoQueryable) to parenthesize each composite boolean node, preserving the expression tree’s intended precedence in SQL.
• Add regression tests (including an EXPLAIN QUERY PLAN assertion) and update README + changelog to document the behavior and breaking-change impact.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
TychoDB/TychoQueryable.cs	Emits &&/`
TychoDB/FilterBuilder.cs	Wraps the caller filter in parentheses so it remains a single conjunct after partition/type predicates.
TychoDB.UnitTests/FilterCompositionTests.cs	Adds multi-partition/type regression tests and an index-usage plan assertion for OR-chains.
README.md	Documents how to explicitly group OR alternatives when mixing with other terms.
CHANGELOG.md	Records the critical fix, LINQ precedence fix, and the resulting breaking-change semantics.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.