From 30169c2db47fb30299925101f6926cd05cb4a8b8 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:20:26 -0500 Subject: [PATCH 1/2] fix: repair the benchmarks build and a flaky identity-hash key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-existing problems that made verification unreliable, both independent of the query work that follows. TychoDB.Benchmarks has not compiled since 123c79f, which added an IJsonSerializer parameter to SortBuilder.Build so property expressions could render against the serializer's member names. One call site in Diagnostics.cs was never updated, so `dotnet build TychoDB.sln` failed outright and nobody could run the benchmarks backing our perf claims. TychoDb_QueryUsingContains_ShouldBeSuccessful keyed 1000 objects by GetHashCode() on a type that does not override it — the runtime identity hash, not a value hash. Instances collide, INSERT OR REPLACE overwrites, and the count assertion fails at random. Measured over 2000 trials of building 1000 TestClassA instances: 16 trials collided (0.80%), with as few as 998 distinct keys. Observed twice as a real suite failure across roughly 20 full runs. Keyed by IntProperty instead, which is already distinct over Range(100, 1000). Co-Authored-By: Claude Opus 5 --- TychoDB.Benchmarks/Diagnostics.cs | 2 +- TychoDB.UnitTests/TychoDbTests.cs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/TychoDB.Benchmarks/Diagnostics.cs b/TychoDB.Benchmarks/Diagnostics.cs index b30ef80..585137b 100644 --- a/TychoDB.Benchmarks/Diagnostics.cs +++ b/TychoDB.Benchmarks/Diagnostics.cs @@ -216,7 +216,7 @@ private static (string Sql, FilterParameters Parameters) BuildSortQuery(Action x.GetHashCode().ToString()).ConfigureAwait(false); + // Keyed by a value that is actually distinct. Object.GetHashCode() is the runtime + // identity hash, and 1000 instances collide about 0.8% of the time (measured over 2000 + // trials); a collision makes INSERT OR REPLACE overwrite, leaving 999 rows and failing + // the count below at random. + await db.WriteObjectsAsync(testObjs, x => x.IntProperty).ConfigureAwait(false); var stopWatch = System.Diagnostics.Stopwatch.StartNew(); From da548af958dbb60e768cc5647b82bfcc2a38249f Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:20:52 -0500 Subject: [PATCH 2/2] fix: bind the caller's filter as a single conjunct, and derive keys honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An ungrouped Or() escaped the partition and type predicates -------------------------------------------------------- FilterBuilder appended the caller's terms bare, so the generated clause read "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. A two-term Or() returned rows from other partitions and rows of other stored types, which the reader then deserialized as T with no error. The same clause backs DeleteObjectsAsync, so an ungrouped Or() could delete rows in other partitions and of other types, and CountObjectsAsync over-counted. The caller's filter is now emitted inside its own parentheses. Losing the Partition predicate also cost the partition-prefixed indexes, so this was a large performance regression too — a grouped OR-chain now uses the index again. TychoQueryable lost precedence inside a predicate ------------------------------------------------- && and || were emitted 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. Each composite boolean node is now emitted in its own group. FilterType.In / NotIn --------------------- Set membership as one atomic term, so it cannot be mis-grouped the way an Or() chain can, rendered through the same numeric CAST the scalar comparisons use so an expression index still serves it. Duplicates are removed; an empty set matches nothing (In) or everything (NotIn), never "IN ()", which is a syntax error, and never a silently dropped term; a null in the set is tested with IS NULL, which SQL's own IN would not do; long lists split across several IN terms rather than exceeding SQLITE_MAX_VARIABLE_NUMBER, which is 999 on older SQLite builds. ReadObjectsByKeysAsync ---------------------- Reads a batch of keys in one round trip, binding the key set as a single JSON array expanded by JSON_EACH rather than one parameter per key, so there is no parameter ceiling and no chunking for callers to think about. Measured against a loop of ReadObjectAsync on a 250,000-row store, best of five after warm-up: 200 keys 1.9ms -> 0.9ms; 999 keys 10.6 -> 4.5; 4,949 keys 36.8 -> 16.9; 23,784 keys 183.2 -> 67.3. Both include deserialization, which dominates the remainder. The shape was chosen by measurement: a single IN collapses at scale (1,297.7ms at 23,784 keys), a chunked IN takes 91.8ms, a temp-table join carries ~40ms of fixed setup. CountObjectsAsync stopped counting rows on the client ----------------------------------------------------- It issued "SELECT 1 FROM JsonValue WHERE ..." and incremented a counter once per matching row, costing a reader round trip per row. It now issues SELECT COUNT(*): 16.0ms -> 6.5ms counting a 250,000-row partition. The same query backs the pre-count a progress-reporting read performs. Key derivation -------------- AddTypeRegistration() documented convention-based ID detection and did none: it recorded no selector, so every by-object call threw "An id mapping has not been provided" on a type whose property was named Id. It now finds Id, then Id, case-insensitively. A type with no such property still registers without a mapping, so registering a key-less type and supplying keys at the call site keeps working. WriteObjectsAsync(objs, keySelector, ...) overrides the registration, and a row written under a key the registration would not produce is unreachable by every by-object overload — DeleteObjectAsync(obj) returns false while the row survives. Under requireTypeRegistration that now throws, naming both keys. The check wraps the selector rather than pre-scanning, so a lazy sequence is enumerated exactly once. That guarantee makes a rewrite sound: under strict registration, Equals and In filters on the id property are answered from the indexed Key column instead of a JSON_EXTRACT scan — 79.3ms -> 0.0ms for Equals, 101.2ms -> 0.2ms for In over 100 keys. Rows already in the database are checked once per type by a divergence probe (~92ms, cached for the connection); a single divergent row disables the rewrite for that type and the ordinary predicate is emitted, so the worst case is the behaviour that was there before. Negations are left alone — they cannot use an index either way — as is a null comparison value, since Key is NOT NULL. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 128 ++++++ README.md | 41 ++ TychoDB.UnitTests/BatchKeyReadTests.cs | 272 +++++++++++ TychoDB.UnitTests/FilterCompositionTests.cs | 308 +++++++++++++ TychoDB.UnitTests/FilterInTests.cs | 477 ++++++++++++++++++++ TychoDB.UnitTests/KeyRegistrationTests.cs | 383 ++++++++++++++++ TychoDB.UnitTests/ZzBatchKeyBench.cs | 467 +++++++++++++++++++ TychoDB/Filter.cs | 12 + TychoDB/FilterBuilder.cs | 441 +++++++++++++++++- TychoDB/KeyColumnRewrite.cs | 71 +++ TychoDB/Queries.cs | 60 ++- TychoDB/RegisteredTypeInformation.cs | 88 +++- TychoDB/Tycho.cs | 278 +++++++++++- TychoDB/TychoQueryable.cs | 31 +- 14 files changed, 3024 insertions(+), 33 deletions(-) create mode 100644 TychoDB.UnitTests/BatchKeyReadTests.cs create mode 100644 TychoDB.UnitTests/FilterCompositionTests.cs create mode 100644 TychoDB.UnitTests/FilterInTests.cs create mode 100644 TychoDB.UnitTests/KeyRegistrationTests.cs create mode 100644 TychoDB.UnitTests/ZzBatchKeyBench.cs create mode 100644 TychoDB/KeyColumnRewrite.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index fed3db3..5a3317a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,28 @@ some query behavior changes (see Breaking changes). ### Fixed +- **Critical: an ungrouped `Or()` escaped the partition and type predicates.** The caller's + filter was appended to the generated `WHERE` clause without being bound as a unit: + + ```sql + WHERE FullTypeName = ? AND Partition = ? AND OR + ``` + + `AND` binds tighter than `OR`, so SQL read that as + `(FullTypeName = ? AND Partition = ? AND term1) OR (term2)` — every term after the first + `Or()` was matched against **the whole table**. A two-term `Or()` returned rows from other + partitions, and rows of *other stored types*, which the reader then deserialized as `T` with + no error. The same clause is used by `DeleteObjectsAsync`, so an ungrouped `Or()` could + **delete rows in other partitions and of other types**, and by `CountObjectsAsync`, which + over-counted. The caller's filter is now emitted inside its own parentheses. Losing the + `Partition` predicate also cost the partition-prefixed indexes, so this was a large + performance regression as well as a correctness one; grouped OR-chains now use the index. + Filters already wrapped in `StartGroup()`/`EndGroup()` were unaffected and still are. +- **LINQ predicates lost their own precedence.** `TychoQueryable` translated `&&` and `||` by + emitting their 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)` — and returned rows matching only `A` despite their failing + `C`. Each composite boolean node is now emitted in its own group. (`.Where(a).Where(b)` + chains were affected the same way when either predicate contained an `||`.) - **Data integrity: filter values are now compared in the form the serializer wrote.** A filter value was rendered with `ToString()`, which is not how the serializer stores it for every type. The clearest case is an enum: both serializers write it as a **number** by @@ -119,6 +141,15 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Performance +- **`CountObjectsAsync` no longer counts rows on the client.** It issued + `SELECT 1 FROM JsonValue WHERE …` and incremented a counter once per matching row, costing a + reader round trip per row. It now issues `SELECT COUNT(*)` and reads the single scalar: + **16.0 ms → 6.5 ms** counting a 250,000-row partition (2.5x). The same query backs the + pre-count a progress-reporting `ReadObjectsAsync` performs, so progress-enabled reads pay + half of what they did. A *filtered* count is still bounded by whether the filtered property + is indexed — counting a 1-in-200 selective filter on an unindexed path takes ~79 ms on the + same store, essentially all of it the `JSON_EXTRACT` scan. + - **`PRAGMA optimize` on connect and disconnect.** `Connect`/`ConnectAsync` and `Disconnect`/`DisconnectAsync`/`Dispose` run SQLite's recommended `PRAGMA optimize` (bounded by `analysis_limit = 400`) so the query planner keeps fresh statistics and @@ -155,6 +186,78 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Added +- **`AddTypeRegistration()` now detects the id property by convention, as documented.** It + previously did nothing of the kind: it recorded no selector, so `WriteObjectAsync(obj)`, + `ReadObjectAsync(obj)`, `ObjectExistsAsync(obj)`, `DeleteObjectAsync(obj)` and `GetIdFor(obj)` + all threw `TychoException: An id mapping has not been provided`, on a type whose property was + literally named `Id`. The property is now found by name — `Id`, then `Id`, matched + case-insensitively, and it must be public, readable and non-indexed. When no such property + exists the type is still registered but without an id mapping, exactly as before, so + registering a key-less type and supplying keys at the call site keeps working. +- **Strict registration now rejects a key that diverges from the registered id property.** + `WriteObjectsAsync(objs, keySelector, …)` takes a key at the call site and overrides the + registration. A row written under a key the registration would not produce is unreachable by + every by-object overload, and the delete failure is silent — `DeleteObjectAsync(obj)` returns + false while the row survives. With `requireTypeRegistration: true` and a type registered by id + property, such a write now throws `TychoException` naming both keys. Delegate registrations + (`AddTypeRegistrationWithCustomKeySelector`) have no property to compare against and are + unaffected, as is everything outside strict mode. The check wraps the selector rather than + pre-scanning, so a lazy sequence is still enumerated exactly once. +- **Filters on the id property are answered from the `Key` column where that is provably + correct.** `Filter(Equals, x => x.Id, …)` and `Filter(In, x => x.Id, …)` previously went + through `JSON_EXTRACT` and scanned. Under `requireTypeRegistration` with a type registered by + id property, they are now emitted against the indexed `Key` column instead: + + | filter on the id property, 250,000 rows | scan | rewritten | + |---|---:|---:| + | `Equals` | 79.3 ms | **0.0 ms** | + | `In`, 100 keys | 101.2 ms | **0.2 ms** | + + Soundness comes from two things together: the write guard above means no row written through + this instance can diverge, and rows already in the database are checked once per type with a + divergence probe before the rewrite is used (~92 ms on that store, on the first such query + only, then cached for the connection). A single divergent row disables the rewrite for that + type and the ordinary predicate is emitted, so the worst case is the behaviour that was there + before. Negated forms (`NotEquals`, `NotIn`) are deliberately left alone — they cannot use an + index either way — as is a null comparison value, since `Key` is `NOT NULL`. +- **`ReadObjectsByKeysAsync(keys, partition, sort, …)`.** Reads a batch of keys in one round + trip. The key set is bound as a **single JSON array** expanded by `JSON_EACH`, not as one + parameter per key, so there is no `SQLITE_MAX_VARIABLE_NUMBER` ceiling (999 on older SQLite + builds), no chunking for callers to think about, and one prepared statement regardless of + batch size. Keys lead the primary key, so each is a primary-key probe. Measured against a + loop of `ReadObjectAsync` on a 250,000-row store (best of five, after warm-up): + + | batch | looped `ReadObjectAsync` | `ReadObjectsByKeysAsync` | + |------:|------------------------:|-------------------------:| + | 200 | 1.9 ms | 0.9 ms | + | 999 | 10.6 ms | 4.5 ms | + | 4,949 | 36.8 ms | 16.9 ms | + |23,784 | 183.2 ms | 67.3 ms | + + That is 2.1–2.7x end to end. Both figures include deserialization, which is identical between + them and dominates what is left — the query alone is 27.5 ms at 23,784 keys. The `JSON_EACH` shape was chosen by + measurement: a single `IN (@p0…@pN)` collapses at scale (1,297.7 ms at 23,784 keys, because + the statement text and plan grow with the batch), a chunked `IN` is 91.8 ms, and a temp-table + join carries ~40 ms of fixed setup. Keys not present are simply absent from the result. +- **`FilterType.In` and `FilterType.NotIn`.** Set membership as a single atomic term, via new + `Filter` overloads taking an `IEnumerable`: + + ```csharp + FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 }); + ``` + + It renders to ` IN (…)` through the same numeric `CAST` the scalar comparisons use, so + an expression index over the property still serves the query. Being one term, it cannot be + mis-grouped the way an `Or()` chain can. Details: + - Duplicate values are removed; the caller's order is preserved. + - An empty set matches nothing for `In` and everything for `NotIn` — never `IN ()`, which is + a syntax error, and never a silently dropped term, which would widen the result set. + - A `null` in the set is matched against a missing or null member with `IS NULL`, which SQL's + own `IN` would never do. `NotIn` keeps SQL's semantics for rows whose member is null: they + are not returned, exactly as `NotEquals` already behaves. + - Longer lists are split across several `IN` terms rather than exceeding + `SQLITE_MAX_VARIABLE_NUMBER`, which is only 999 on older SQLite builds, so a large set works + regardless of which build the host application ships. - **`IJsonValueResolver`.** A second optional serializer capability, feature-detected the same way, reporting the scalar form a CLR value takes in JSON so filter comparisons are made against what was stored. Implemented by `SystemTextJsonSerializer` and @@ -169,6 +272,21 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Breaking changes +- **`AddTypeRegistration()` on a type with a conventional id property now supplies a key.** + Previously every by-object operation on such a type threw; they now work. Code that caught + that exception, or that relied on `WriteObjectsAsync(objs, keySelector)` disagreeing with a + conventionally-named `Id` property, changes behaviour — under `requireTypeRegistration` the + disagreement is now an error rather than a silently unreachable row. + +- **An ungrouped `Or()` now means what it reads as.** Code that (unknowingly) depended on the + leaked rows — most plausibly a query written against a single-partition, single-type database + where the bug was invisible — returns fewer rows now. This is the fix, not a regression. +- **Passing a collection to a scalar `FilterType` now throws `ArgumentException`.** Adding the + `IEnumerable` overloads changes overload resolution for a collection argument, which + previously bound to `object` and was rendered as `ToString()` (`"System.Int32[]"`), matching + nothing silently. Use `FilterType.In`. A literal `null` argument also now binds to the new + overload, but keeps its old meaning — `Filter(Equals, x => x.Value, null)` is still the + null comparison. - **Enum, `DateOnly` and `TimeOnly` filter values now compare against their JSON form.** Code that worked around the enum mismatch by casting to `(int)` keeps working. Code that relied on a string-enum converter's name matching by coincidence also keeps working, and now stays @@ -215,6 +333,16 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Notes +- **Serializer choice is the largest remaining lever on read throughput.** Reading a whole + 250,000-row partition measured 254.7 ms with `SystemTextJsonSerializer` against 358.9 ms with + `NewtonsoftJsonSerializer` (~1.4x), because the former implements `IUtf8JsonDeserializer` and + receives rows as UTF-8 spans. Deserialization dominates any large read: of the 67.3 ms + `ReadObjectsByKeysAsync` takes for 23,784 keys, only 27.5 ms is the query. +- **Outside strict mode, a filter on the id property still scans.** The `Key`-column rewrite + needs the write guard to hold, and that guard only applies under `requireTypeRegistration`. + Without it, index the id property or reach those rows through `ReadObjectsByKeysAsync`; both + measured 0.0 ms against the 71.6 ms scan. + - Performance guidance: prefer `WriteObjectsAsync` for writing many objects — it is ~10× faster and ~6× lower-allocation than looping `WriteObjectAsync`, and `withTransaction: true` is faster than `false` for bulk writes. diff --git a/README.md b/README.md index 5431d33..8452fbe 100644 --- a/README.md +++ b/README.md @@ -190,8 +190,20 @@ var exists = await db.ObjectExistsAsync("123"); // Count objects var count = await db.CountObjectsAsync(); + +// Read many objects by key in one round trip. Prefer this over a loop of ReadObjectAsync: +// keys lead the primary key, and there is no limit on how many may be passed. +var people = await db.ReadObjectsByKeysAsync(new object[] { "id-1", "id-2", "id-3" }); ``` +> **Filtering on the property that is also the Tycho key** (`x => x.Id`) normally goes through +> `JSON_EXTRACT` and scans, because a write may supply its own key selector and Tycho cannot +> assume the property still matches the stored key. Under `requireTypeRegistration: true`, with +> the type registered by id property, that assumption *is* enforced, and `Equals` / `In` filters +> on the id property are answered from the indexed `Key` column instead — 79.3 ms to 0.0 ms on a +> 250,000-row store. Otherwise, reach those rows through `ReadObjectAsync` / +> `ReadObjectsByKeysAsync`, or index the property like any other. + ### Filtering ```csharp @@ -210,6 +222,22 @@ var complexFilter = FilterBuilder .And() .Filter(FilterType.Contains, x => x.Name, "Doe"); +// Set membership: one atomic term, so it needs no grouping +var inDepartments = FilterBuilder + .Create() + .Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47, 51 }); + +// Mixing OR with other terms: group the alternatives so the intent is explicit +var grouped = FilterBuilder + .Create() + .StartGroup() + .Filter(FilterType.Equals, x => x.DepartmentId, 33) + .Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47) + .EndGroup() + .And() + .Filter(FilterType.GreaterThan, x => x.Age, 25); + // Get a single object matching the filter var johnDoe = await db.ReadObjectAsync(filter: complexFilter); @@ -323,6 +351,19 @@ var result = await db.DeleteBlobsAsync("documents"); Console.WriteLine($"Deleted {result.Count} blobs"); ``` +## Performance notes + +- **Reach rows by key through the key APIs.** A filter on the key property scans; see the note + under [Basic Querying](#basic-querying). `ReadObjectsByKeysAsync` fetches a whole batch in one + round trip and has no limit on batch size. +- **`SystemTextJsonSerializer` deserializes faster.** It implements `IUtf8JsonDeserializer`, so + rows are handed to it as UTF-8 spans and skip an intermediate stream. Reading a whole + 250,000-row partition measured **254.7 ms** with `SystemTextJsonSerializer` against + **358.9 ms** with `NewtonsoftJsonSerializer` — about 1.4x. Deserialization dominates any large + read, so this is usually the largest single lever on read throughput. +- **Index anything you filter or sort on.** An unindexed `JSON_EXTRACT` predicate scans the + partition; see below. + ## Indexing Create indexes to improve query performance: diff --git a/TychoDB.UnitTests/BatchKeyReadTests.cs b/TychoDB.UnitTests/BatchKeyReadTests.cs new file mode 100644 index 0000000..b71b022 --- /dev/null +++ b/TychoDB.UnitTests/BatchKeyReadTests.cs @@ -0,0 +1,272 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; + +namespace TychoDB.UnitTests; + +/// +/// Reading a batch of keys in one round trip. The key set is bound as a JSON array rather than +/// one parameter per key, so the tests that matter most are the ones that would break that +/// encoding: keys carrying JSON metacharacters, and key sets larger than SQLite's parameter +/// ceiling. +/// +[TestClass] +public class BatchKeyReadTests +{ + private const string PartitionA = "partitionA"; + private const string PartitionB = "partitionB"; + + [TestMethod] + public async Task ReadsTheObjectsForTheGivenKeys() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "k3" }, PartitionA); + + results.Select(x => x.Key).OrderBy(x => x).ShouldBe(new[] { "k1", "k3" }); + } + + [TestMethod] + public async Task KeysThatAreNotPresent_AreSimplyAbsent() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "nope", "k2" }, PartitionA); + + results.Select(x => x.Key).OrderBy(x => x).ShouldBe(new[] { "k1", "k2" }); + } + + [TestMethod] + public async Task EmptyKeySet_ReturnsNothing() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(Array.Empty(), PartitionA); + + results.ShouldBeEmpty(); + } + + [TestMethod] + public async Task StaysWithinItsPartition() + { + using var db = Connect(); + await SeedAsync(db); + + // Both keys exist in both partitions, holding different values; only partition A's + // versions may come back. + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "shared" }, PartitionA); + + results.Select(x => x.Description).OrderBy(x => x, StringComparer.Ordinal) + .ShouldBe(new[] { "a-shared", "a1" }); + } + + [TestMethod] + public async Task StaysWithinItsType() + { + // Same key, same partition, different stored type. + using var db = Connect(); + await SeedAsync(db); + await db.WriteObjectsAsync(new[] { new Other { Key = "k1", Note = "wrong type" } }, x => x.Key, PartitionA); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1" }, PartitionA); + + results.Select(x => x.Description).ShouldBe(new[] { "a1" }); + } + + [TestMethod] + public async Task DuplicateKeys_YieldOneObjectEach() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "k1", "k1" }, PartitionA); + + results.Select(x => x.Key).ShouldBe(new[] { "k1" }); + } + + [TestMethod] + public async Task KeysCarryingJsonMetacharacters_RoundTrip() + { + // The key set is rendered as JSON, so a quote, a backslash, a control character or a + // non-BMP character in a key would corrupt the array if it were not properly encoded. + var hostile = new[] + { + "he said \"hi\"", + @"back\slash", + "tab\tand\nnewline", + "emoji \U0001F389", + "unicode é中", + "'; DROP TABLE JsonValue; --", + }; + + using var db = Connect(); + await db.WriteObjectsAsync( + hostile.Select(k => new Doc { Key = k, Description = "held" }), x => x.Key, PartitionA); + + var results = await db.ReadObjectsByKeysAsync(hostile.Cast(), PartitionA); + + results.Select(x => x.Key).OrderBy(x => x, StringComparer.Ordinal) + .ShouldBe(hostile.OrderBy(x => x, StringComparer.Ordinal)); + } + + [TestMethod] + public async Task KeySetLargerThanTheParameterCeiling_Works() + { + // One bound parameter regardless of key count, so SQLITE_MAX_VARIABLE_NUMBER — 999 on + // older builds — does not apply and no chunking is needed. + const int count = 5_000; + + using var db = Connect(); + await db.WriteObjectsAsync( + Enumerable.Range(0, count).Select(i => new Doc + { + Key = "b" + i.ToString(CultureInfo.InvariantCulture), + Description = "bulk", + }), + x => x.Key, + PartitionA); + + var keys = Enumerable.Range(0, count).Select(i => (object)("b" + i.ToString(CultureInfo.InvariantCulture))); + + var results = await db.ReadObjectsByKeysAsync(keys, PartitionA); + + results.Count().ShouldBe(count); + } + + [TestMethod] + public async Task NonStringKeys_MatchTheSingleKeyOverload() + { + using var db = Connect(); + await db.WriteObjectsAsync( + new[] { new Numbered { Id = 7, Note = "seven" }, new Numbered { Id = 8, Note = "eight" } }, + x => x.Id, + PartitionA); + + var batch = await db.ReadObjectsByKeysAsync(new object[] { 7, 8 }, PartitionA); + var single = await db.ReadObjectAsync(7, PartitionA); + + batch.Select(x => x.Note).OrderBy(x => x).ShouldBe(new[] { "eight", "seven" }); + single.Note.ShouldBe("seven"); + } + + [TestMethod] + public async Task AppliesSorting() + { + using var db = Connect(); + await SeedAsync(db); + + var results = + await db.ReadObjectsByKeysAsync( + new object[] { "k1", "k2", "k3" }, + PartitionA, + SortBuilder.Create().OrderBy(SortDirection.Descending, x => x.Description)); + + results.Select(x => x.Description).ShouldBe(new[] { "a3", "a2", "a1" }); + } + + [TestMethod] + public async Task ReportsProgress() + { + using var db = Connect(); + await SeedAsync(db); + + // A synchronous reporter, not Progress, which dispatches its callbacks + // asynchronously and would make this a race. + var progress = new RecordingProgress(); + + await db.ReadObjectsByKeysAsync(new object[] { "k1", "k2", "k3" }, PartitionA, progress: progress); + + progress.Reports.ShouldNotBeEmpty(); + progress.Reports[^1].ShouldBe(1.0); + } + + [TestMethod] + public async Task NullKeyInTheSet_IsRejected() + { + using var db = Connect(); + await SeedAsync(db); + + await Should.ThrowAsync( + async () => await db.ReadObjectsByKeysAsync(new object?[] { "k1", null }!, PartitionA)); + } + + [TestMethod] + public async Task NullKeySet_IsRejected() + { + using var db = Connect(); + await SeedAsync(db); + + await Should.ThrowAsync( + async () => await db.ReadObjectsByKeysAsync(null!, PartitionA)); + } + + private static Tycho Connect() + { + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + + var db = new Tycho(dir, new NewtonsoftJsonSerializer(), dbName: name, rebuildCache: true, requireTypeRegistration: false); + return db.Connect(); + } + + private static async Task SeedAsync(Tycho db) + { + await db.WriteObjectsAsync( + new[] + { + new Doc { Key = "k1", Description = "a1" }, + new Doc { Key = "k2", Description = "a2" }, + new Doc { Key = "k3", Description = "a3" }, + new Doc { Key = "shared", Description = "a-shared" }, + }, + x => x.Key, + PartitionA); + + await db.WriteObjectsAsync( + new[] + { + new Doc { Key = "k1", Description = "b1" }, + new Doc { Key = "shared", Description = "b-shared" }, + }, + x => x.Key, + PartitionB); + } + + private sealed class RecordingProgress : IProgress + { + public List Reports { get; } = new(); + + public void Report(double value) => Reports.Add(value); + } + + public class Doc + { + public string Key { get; set; } = string.Empty; + + public string Description { get; set; } = string.Empty; + } + + public class Other + { + public string Key { get; set; } = string.Empty; + + public string Note { get; set; } = string.Empty; + } + + public class Numbered + { + public int Id { get; set; } + + public string Note { get; set; } = string.Empty; + } +} diff --git a/TychoDB.UnitTests/FilterCompositionTests.cs b/TychoDB.UnitTests/FilterCompositionTests.cs new file mode 100644 index 0000000..a474ae2 --- /dev/null +++ b/TychoDB.UnitTests/FilterCompositionTests.cs @@ -0,0 +1,308 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Data.Sqlite; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; + +namespace TychoDB.UnitTests; + +/// +/// The caller's filter is only one conjunct of the generated WHERE clause — the partition and +/// type predicates are the others. Because AND binds tighter than OR, an ungrouped OR in the +/// caller's filter splits the clause and its trailing terms escape both predicates, matching +/// rows of other partitions and other stored types. Every test here therefore seeds more than +/// one partition and more than one type: a single-partition, single-type fixture cannot observe +/// the difference. +/// +[TestClass] +public class FilterCompositionTests +{ + private const string PartitionA = "partitionA"; + private const string PartitionB = "partitionB"; + private const string Shared = "shared"; + + [TestMethod] + public async Task UngroupedOr_DoesNotMatchRowsInOtherPartitions() + { + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, PartitionA); + await db.WriteObjectsAsync( + new[] { Item(3, 33), Item(4, 47) }, x => x.Id, PartitionB); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47)); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public async Task UngroupedOr_DoesNotMatchRowsOfOtherTypes() + { + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, Shared); + await db.WriteObjectsAsync( + new[] { new VendorModel { Id = 900, Description = "VENDOR" } }, + x => x.Id.ToString(), + Shared); + + var results = + await db.ReadObjectsAsync( + Shared, + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Contains, x => x.Description, "VENDOR")); + + results.Select(x => x.Id).ShouldBe(new[] { 1 }); + } + + [TestMethod] + public async Task UngroupedOr_DoesNotDeleteRowsInOtherPartitions() + { + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, PartitionA); + await db.WriteObjectsAsync( + new[] { Item(3, 33), Item(4, 47) }, x => x.Id, PartitionB); + + var deleted = + await db.DeleteObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47)); + + deleted.ShouldBe(2); + + var survivors = await db.ReadObjectsAsync(PartitionB); + survivors.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 3, 4 }); + } + + [TestMethod] + public async Task UngroupedOr_DoesNotCountRowsInOtherPartitions() + { + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, PartitionA); + await db.WriteObjectsAsync( + new[] { Item(3, 33), Item(4, 47) }, x => x.Id, PartitionB); + + var count = + await db.CountObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47)); + + count.ShouldBe(2); + } + + [TestMethod] + public async Task ExplicitlyGroupedOr_StillMatchesOnlyItsOwnPartition() + { + // The grouped form was already correct; it must stay correct once the builder adds its + // own enclosing parentheses. + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, PartitionA); + await db.WriteObjectsAsync( + new[] { Item(3, 33), Item(4, 47) }, x => x.Id, PartitionB); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .StartGroup() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47) + .EndGroup()); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public async Task AndChain_IsUnaffected() + { + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, PartitionA); + await db.WriteObjectsAsync( + new[] { Item(3, 33) }, x => x.Id, PartitionB); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).And() + .Filter(FilterType.Equals, x => x.Description, "ITEM")); + + results.Select(x => x.Id).ShouldBe(new[] { 1 }); + } + + [TestMethod] + public async Task Linq_PrecedenceWithinPredicate_IsPreserved() + { + // (a || b) && c must not be flattened into "a OR b AND c", which SQL reads as + // "a OR (b AND c)" — an item matching only `a` then comes back despite failing `c`. + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] + { + new ItemModel { Id = 1, DepartmentId = 33, Description = "OTHER" }, + new ItemModel { Id = 2, DepartmentId = 47, Description = "ITEM" }, + }, + x => x.Id, + PartitionA); + + var results = + await db.Query(PartitionA) + .Where(x => (x.DepartmentId == 33 || x.DepartmentId == 47) && x.Description == "ITEM") + .ToListAsync(); + + results.Select(x => x.Id).ShouldBe(new[] { 2 }); + } + + [TestMethod] + public async Task Linq_OrPredicate_DoesNotMatchRowsInOtherPartitions() + { + using var db = Connect(); + + await db.WriteObjectsAsync( + new[] { Item(1, 33), Item(2, 47) }, x => x.Id, PartitionA); + await db.WriteObjectsAsync( + new[] { Item(3, 33), Item(4, 47) }, x => x.Id, PartitionB); + + var results = + await db.Query(PartitionA) + .Where(x => x.DepartmentId == 33 || x.DepartmentId == 47) + .ToListAsync(); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public async Task UngroupedOr_OnAnIndexedProperty_UsesTheIndex() + { + // The correctness bug had a performance face: the escaped terms lost the Partition + // predicate too, so they could not use the partition-prefixed index and fell back to + // scanning the whole table. + var db = Connect(out var path); + await db.WriteObjectsAsync( + Enumerable.Range(1, 2_000).Select(i => Item(i, i)), x => x.Id, PartitionA); + await db.CreateIndexAsync(x => x.DepartmentId, "ix_or_department"); + + db.Dispose(); + SqliteConnection.ClearAllPools(); + + var plan = + Explain( + path, + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47)); + + plan.ShouldContain("ix_or_department"); + plan.ShouldNotContain("SCAN JsonValue", Case.Sensitive); + } + + [TestMethod] + public void GeneratedSql_BindsTheCallerFilterAsASingleConjunct() + { + var sb = new StringBuilder(Queries.SelectDataFromJsonValueWithFullTypeName); + + FilterBuilder.Create() + .Filter(FilterType.Equals, x => x.DepartmentId, 33).Or() + .Filter(FilterType.Equals, x => x.DepartmentId, 47) + .Build(sb, Serializer, new FilterParameters()); + + var sql = string.Join(' ', sb.ToString().Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)); + + System.Console.WriteLine(sql); + sql.ShouldContain("Partition = $partition AND ("); + sql.ShouldEndWith(")"); + } + + private static string Explain(string dbFile, FilterBuilder filter) + { + var sb = new StringBuilder(Queries.SelectDataFromJsonValueWithFullTypeName); + var parameters = new FilterParameters(); + filter.Build(sb, Serializer, parameters); + + using var conn = new SqliteConnection($"Data Source={dbFile}"); + conn.Open(); + using var command = conn.CreateCommand(); + +#pragma warning disable CA2100 // SQL is produced by the library's own builders. + command.CommandText = "EXPLAIN QUERY PLAN " + sb.ToString(); +#pragma warning restore CA2100 + + command.Parameters.AddWithValue("$fullTypeName", typeof(ItemModel).FullName); + command.Parameters.AddWithValue("$partition", PartitionA); + for (var i = 0; i < parameters.Count; i++) + { + command.Parameters.AddWithValue( + FilterParameters.ParameterPrefix + i.ToString(System.Globalization.CultureInfo.InvariantCulture), + parameters.Values[i] ?? (object)DBNull.Value); + } + + var plan = new StringBuilder(); + using var reader = command.ExecuteReader(); + while (reader.Read()) + { + plan.AppendLine(reader.GetString(reader.FieldCount - 1)); + } + + return plan.ToString(); + } + + private static ItemModel Item(int id, int departmentId) => + new() { Id = id, DepartmentId = departmentId, Description = "ITEM" }; + + private static readonly IJsonSerializer Serializer = new NewtonsoftJsonSerializer(); + + private static Tycho Connect() => Connect(out _); + + private static Tycho Connect(out string path) + { + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + path = Path.Combine(dir, name); + + var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: true, requireTypeRegistration: false); + return db.Connect(); + } + + public class ItemModel + { + public int Id { get; set; } + + public int DepartmentId { get; set; } + + public string? Description { get; set; } + } + + public class VendorModel + { + public int Id { get; set; } + + public string? Description { get; set; } + } +} diff --git a/TychoDB.UnitTests/FilterInTests.cs b/TychoDB.UnitTests/FilterInTests.cs new file mode 100644 index 0000000..95098ff --- /dev/null +++ b/TychoDB.UnitTests/FilterInTests.cs @@ -0,0 +1,477 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Threading.Tasks; +using Microsoft.Data.Sqlite; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; + +namespace TychoDB.UnitTests; + +/// +/// Set membership as a single atomic term. The point of it over a chain of Or()s is that it +/// cannot be mis-grouped, so these tests assert the results themselves rather than the SQL — +/// except where the shape is the behaviour under test (the numeric CAST that keeps an +/// expression index usable, and the chunking that keeps a long list under SQLite's parameter +/// ceiling). +/// +[TestClass] +public class FilterInTests +{ + private const string PartitionA = "partitionA"; + private const string PartitionB = "partitionB"; + + [TestMethod] + public async Task In_MatchesOnlyTheListedValues() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public async Task NotIn_MatchesEverythingElse() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.NotIn, x => x.DepartmentId, new[] { 33, 47 })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 3 }); + } + + [TestMethod] + public async Task In_StaysWithinItsPartition() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionB, + FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 })); + + results.Select(x => x.Id).ShouldBe(new[] { 4 }); + } + + [TestMethod] + public async Task In_OnStringProperty_MatchesRows() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.Description, new[] { "alpha", "gamma" })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 3 }); + } + + [TestMethod] + public async Task In_OnEnumProperty_ComparesTheSerializedForm() + { + // The set has to be resolved element by element, the way a scalar comparison value is: + // with a string-enum converter the stored form is the member name, not the number. + var serializer = new SystemTextJsonSerializer( + new JsonSerializerOptions { Converters = { new JsonStringEnumConverter() } }); + + using var db = Connect(out _, serializer); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.In, x => x.Allocation, new[] { Allocation.Produce, Allocation.Dairy })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public async Task In_OnDateTimeProperty_MatchesRows() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.In, x => x.Created, new[] { BaseDate, BaseDate.AddDays(2) })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 3 }); + } + + [TestMethod] + public async Task In_OnBoolProperty_MatchesRows() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.IsActive, new[] { true })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 3 }); + } + + [TestMethod] + public async Task In_WithEmptySet_MatchesNothing() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, Array.Empty())); + + results.ShouldBeEmpty(); + } + + [TestMethod] + public async Task NotIn_WithEmptySet_MatchesEverything() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.NotIn, x => x.DepartmentId, Array.Empty())); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2, 3 }); + } + + [TestMethod] + public async Task In_WithNullInTheSet_MatchesMissingValues() + { + // SQL's own IN never matches NULL against a NULL in the list; the builder pulls the + // null out into an IS NULL test so the caller's intent survives. + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.Description, new[] { "alpha", null })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public async Task In_WithDuplicates_MatchesTheSameRowsOnce() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 33, 47, 33 })); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public void In_WithDuplicates_RendersEachValueOnce() + { + var sql = Render(f => f.Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 33, 47, 33 }), out _); + + sql.ShouldContain("IN (33, 47)"); + } + + [TestMethod] + public void In_OnNumericProperty_KeepsTheCastThatMakesAnIndexUsable() + { + var sql = Render(f => f.Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 }), out _); + + sql.ShouldContain("CAST(JSON_EXTRACT(Data, '$.DepartmentId') as NUMERIC) IN (33, 47)"); + } + + [TestMethod] + public async Task In_OnAnIndexedNumericProperty_UsesTheIndex() + { + // The whole point of rendering the numeric CAST is that SQLite matches expression + // indexes structurally; get the form wrong and this silently degrades to a table scan. + // Enough rows are seeded that the planner has a reason to prefer the index. + var db = Connect(out var path); + await db.WriteObjectsAsync( + Enumerable.Range(1, 2_000) + .Select(i => new Doc { Id = i, DepartmentId = i, Description = "d" + i.ToString(CultureInfo.InvariantCulture) }), + x => x.Id, + PartitionA); + await db.CreateIndexAsync(x => x.DepartmentId, "ix_in_department"); + + db.Dispose(); + SqliteConnection.ClearAllPools(); + + var plan = ExplainFilter(path, f => f.Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 })); + + plan.ShouldContain("ix_in_department"); + plan.ShouldNotContain("SCAN JsonValue", Case.Sensitive); + } + + [TestMethod] + public async Task In_WithMoreValuesThanTheParameterCeiling_StillMatches() + { + // Long lists are split across several IN terms rather than exceeding + // SQLITE_MAX_VARIABLE_NUMBER, which is 999 on older builds. + using var db = Connect(out _); + await SeedAsync(db); + + var manyStrings = + Enumerable.Range(0, 2_500).Select(i => "filler" + i.ToString(CultureInfo.InvariantCulture)) + .Concat(new[] { "alpha", "gamma" }) + .ToArray(); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.Description, manyStrings)); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 3 }); + } + + [TestMethod] + public async Task NotIn_WithMoreValuesThanTheParameterCeiling_StillMatches() + { + using var db = Connect(out _); + await SeedAsync(db); + + var manyStrings = + Enumerable.Range(0, 2_500).Select(i => "filler" + i.ToString(CultureInfo.InvariantCulture)) + .Concat(new[] { "gamma" }) + .ToArray(); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.NotIn, x => x.Description, manyStrings)); + + // Id 2 has no description: NULL fails NOT IN, exactly as it fails NotEquals. + results.Select(x => x.Id).ShouldBe(new[] { 1 }); + } + + [TestMethod] + public async Task NotIn_ExcludesNullValues_LikeNotEquals() + { + // Pinning the SQL NULL semantics deliberately rather than by accident: a row whose + // member is absent or null is not returned by NotIn, which is how the scalar + // NotEquals already behaves. Callers who want those rows add an explicit + // Or(Equals(path, null)) term. + using var db = Connect(out _); + await SeedAsync(db); + + var notIn = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.NotIn, x => x.Description, new[] { "alpha" })); + + var notEquals = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.NotEquals, x => x.Description, "alpha")); + + notIn.Select(x => x.Id).ShouldBe(new[] { 3 }); + notIn.Select(x => x.Id).ShouldBe(notEquals.Select(x => x.Id)); + } + + [TestMethod] + public async Task In_ComposesWithOtherTerms() + { + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create() + .Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 }).And() + .Filter(FilterType.Equals, x => x.IsActive, true)); + + results.Select(x => x.Id).ShouldBe(new[] { 1 }); + } + + [TestMethod] + public async Task In_UsedForDeletion_RemovesOnlyItsOwnPartition() + { + using var db = Connect(out _); + await SeedAsync(db); + + var deleted = + await db.DeleteObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 })); + + deleted.ShouldBe(2); + (await db.ReadObjectsAsync(PartitionB)).Select(x => x.Id).ShouldBe(new[] { 4 }); + } + + [TestMethod] + public void SetFilterType_OnTheScalarOverload_IsRejected() + { + var build = () => FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, (object)33); + + Should.Throw(build).Message.ShouldContain("IEnumerable"); + } + + [TestMethod] + public async Task NullValue_StillMeansTheScalarNullComparison() + { + // Adding an IEnumerable overload changes where a literal null binds: it is the + // more specific parameter type, so Filter(Equals, x => x.Description, null) now lands on + // the collection overload. It has to keep meaning "compare against null". + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter(FilterType.Equals, x => x.Description, null)); + + results.Select(x => x.Id).ShouldBe(new[] { 2 }); + } + + [TestMethod] + public void NullValue_WithASetFilterType_IsRejected() + { + // The empty set is a meaningful request; a missing set is not. + var build = () => FilterBuilder.Create().Filter(FilterType.In, x => x.Description, null); + + Should.Throw(build).Message.ShouldContain("empty one"); + } + + [TestMethod] + public void ScalarFilterType_OnTheCollectionOverload_IsRejected() + { + var build = () => FilterBuilder.Create().Filter(FilterType.Equals, x => x.DepartmentId, new[] { 33, 47 }); + + Should.Throw(build).Message.ShouldContain("collection"); + } + + private static readonly DateTime BaseDate = new(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + private static readonly IJsonSerializer DefaultSerializer = new NewtonsoftJsonSerializer(); + + private static string Render(Action> configure, out FilterParameters parameters) + { + var filter = FilterBuilder.Create(); + configure(filter); + + var sb = new StringBuilder(); + parameters = new FilterParameters(); + filter.Build(sb, DefaultSerializer, parameters); + + return sb.ToString(); + } + + private static string ExplainFilter(string dbFile, Action> configure) + { + var filter = FilterBuilder.Create(); + configure(filter); + + var sb = new StringBuilder(Queries.SelectDataFromJsonValueWithFullTypeName); + var parameters = new FilterParameters(); + filter.Build(sb, DefaultSerializer, parameters); + + using var conn = new SqliteConnection($"Data Source={dbFile}"); + conn.Open(); + using var command = conn.CreateCommand(); + +#pragma warning disable CA2100 // SQL is produced by the library's own builders. + command.CommandText = "EXPLAIN QUERY PLAN " + sb.ToString(); +#pragma warning restore CA2100 + + command.Parameters.AddWithValue("$fullTypeName", typeof(Doc).FullName); + command.Parameters.AddWithValue("$partition", PartitionA); + for (var i = 0; i < parameters.Count; i++) + { + command.Parameters.AddWithValue( + FilterParameters.ParameterPrefix + i.ToString(CultureInfo.InvariantCulture), + parameters.Values[i] ?? (object)DBNull.Value); + } + + var plan = new StringBuilder(); + using var reader = command.ExecuteReader(); + while (reader.Read()) + { + plan.AppendLine(reader.GetString(reader.FieldCount - 1)); + } + + return plan.ToString(); + } + + private static Tycho Connect(out string path, IJsonSerializer? jsonSerializer = null) + { + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + path = Path.Combine(dir, name); + + var db = new Tycho( + dir, jsonSerializer ?? DefaultSerializer, dbName: name, rebuildCache: true, requireTypeRegistration: false); + return db.Connect(); + } + + private static async Task SeedAsync(Tycho db) + { + await db.WriteObjectsAsync( + new[] + { + new Doc { Id = 1, DepartmentId = 33, Description = "alpha", IsActive = true, Created = BaseDate, Allocation = Allocation.Produce }, + new Doc { Id = 2, DepartmentId = 47, Description = null, IsActive = false, Created = BaseDate.AddDays(1), Allocation = Allocation.Dairy }, + new Doc { Id = 3, DepartmentId = 51, Description = "gamma", IsActive = true, Created = BaseDate.AddDays(2), Allocation = Allocation.Bakery }, + }, + x => x.Id, + PartitionA); + + await db.WriteObjectsAsync( + new[] + { + new Doc { Id = 4, DepartmentId = 33, Description = "alpha", IsActive = true, Created = BaseDate, Allocation = Allocation.Produce }, + }, + x => x.Id, + PartitionB); + } + + public enum Allocation + { + Produce = 0, + Dairy = 1, + Bakery = 2, + } + + public class Doc + { + public int Id { get; set; } + + public int DepartmentId { get; set; } + + public string? Description { get; set; } + + public bool IsActive { get; set; } + + public DateTime Created { get; set; } + + public Allocation Allocation { get; set; } + } +} diff --git a/TychoDB.UnitTests/KeyRegistrationTests.cs b/TychoDB.UnitTests/KeyRegistrationTests.cs new file mode 100644 index 0000000..15c4e59 --- /dev/null +++ b/TychoDB.UnitTests/KeyRegistrationTests.cs @@ -0,0 +1,383 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; +using Microsoft.Data.Sqlite; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; + +namespace TychoDB.UnitTests; + +/// +/// How a row's key is decided, and what follows from it: convention-based registration, the +/// strict-mode guard that stops a write from producing a key the by-object overloads cannot +/// reach, and the key-column rewrite those two together make sound. +/// +[TestClass] +public class KeyRegistrationTests +{ + private const string Partition = "p"; + + // ---------- convention-based registration ---------- + [TestMethod] + public async Task ConventionRegistration_FindsAnIdProperty() + { + using var db = Connect(t => t.AddTypeRegistration()); + + var doc = new Doc { Id = "id-1", Value = "v" }; + await db.WriteObjectAsync(doc, Partition); + + (await db.ReadObjectAsync(doc, Partition)).Value.ShouldBe("v"); + db.GetIdFor(doc).ShouldBe("id-1"); + } + + [TestMethod] + public async Task ConventionRegistration_FindsTypeNameIdProperty() + { + using var db = Connect(t => t.AddTypeRegistration()); + + var widget = new Widget { WidgetId = 7, Value = "w" }; + await db.WriteObjectAsync(widget, Partition); + + (await db.ReadObjectAsync(widget, Partition)).Value.ShouldBe("w"); + } + + [TestMethod] + public async Task ConventionRegistration_PrefersIdOverTypeNameId() + { + using var db = Connect(t => t.AddTypeRegistration()); + + db.GetIdFor(new Both { Id = "plain", BothId = "prefixed" }).ShouldBe("plain"); + } + + [TestMethod] + public async Task ConventionRegistration_WithNoIdProperty_StillAllowsExplicitKeys() + { + // The pre-existing behaviour has to survive: registering a key-less type is how a caller + // satisfies requireTypeRegistration while supplying keys at the call site. + using var db = Connect(t => t.AddTypeRegistration()); + + await db.WriteObjectsAsync(new[] { new Keyless { Value = "v" } }, x => "explicit", Partition); + + (await db.ReadObjectAsync("explicit", Partition)).Value.ShouldBe("v"); + Should.Throw(() => db.GetIdFor(new Keyless())).Message.ShouldContain("id mapping"); + } + + [TestMethod] + public async Task ConventionRegistration_IgnoresAnIndexer() + { + // An indexer cannot become a property path; the type must fall back to explicit keys. + using var db = Connect(t => t.AddTypeRegistration()); + + Should.Throw(() => db.GetIdFor(new Indexed())).Message.ShouldContain("id mapping"); + await Task.CompletedTask; + } + + // ---------- strict-mode divergence guard ---------- + [TestMethod] + public async Task StrictMode_RejectsAKeySelectorThatDisagreesWithTheRegistration() + { + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + + var ex = await Should.ThrowAsync( + async () => await db.WriteObjectsAsync( + new[] { new Doc { Id = "id-1", Value = "v" } }, x => "custom-" + x.Id, Partition)); + + // The guard fires while the sequence is being enumerated inside the write, so it + // arrives wrapped in the write path's usual TychoException, as every write failure does. + var guard = ex.InnerException.ShouldBeOfType(); + guard.Message.ShouldContain("custom-id-1"); + guard.Message.ShouldContain("registered id property"); + + // Nothing was written under either key. + (await db.ReadObjectsAsync(Partition)).ShouldBeEmpty(); + } + + [TestMethod] + public async Task StrictMode_AllowsAKeySelectorThatAgrees() + { + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + + await db.WriteObjectsAsync(new[] { new Doc { Id = "id-1", Value = "v" } }, x => x.Id, Partition); + + (await db.ReadObjectAsync(new Doc { Id = "id-1" }, Partition)).Value.ShouldBe("v"); + } + + [TestMethod] + public async Task StrictMode_WithADelegateRegistration_DoesNotGuard() + { + // A delegate registration has no id property to compare against, so the override stays + // available exactly as before. + using var db = Connect(t => t.AddTypeRegistrationWithCustomKeySelector(x => x.Id), strict: true); + + await db.WriteObjectsAsync(new[] { new Doc { Id = "id-1", Value = "v" } }, x => "anything", Partition); + + (await db.ReadObjectAsync("anything", Partition)).Value.ShouldBe("v"); + } + + [TestMethod] + public async Task OutsideStrictMode_TheOverrideIsStillPermitted() + { + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: false); + + await db.WriteObjectsAsync(new[] { new Doc { Id = "id-1", Value = "v" } }, x => "custom-" + x.Id, Partition); + + (await db.ReadObjectAsync("custom-id-1", Partition)).Value.ShouldBe("v"); + } + + [TestMethod] + public async Task Guard_EnumeratesTheSequenceOnlyOnce() + { + // The guard wraps the selector rather than pre-scanning, because callers pass lazy + // sequences that must not be enumerated twice. + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + + var enumerations = 0; + + IEnumerable Lazy() + { + enumerations++; + yield return new Doc { Id = "id-1", Value = "v" }; + } + + await db.WriteObjectsAsync(Lazy(), x => x.Id, Partition); + + enumerations.ShouldBe(1); + } + + // ---------- key-column rewrite ---------- + [TestMethod] + public async Task KeyPropertyFilter_UsesTheKeyColumn_InStrictMode() + { + var (db, path) = ConnectAt(t => t.AddTypeRegistration(x => x.Id), strict: true); + using var scoped = db; + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Id, "id-2")); + + results.Select(x => x.Value).ShouldBe(new[] { "v2" }); + (await PlanFor(db, path)).ShouldNotContain("SCAN JsonValue", Case.Sensitive); + } + + [TestMethod] + public async Task KeyPropertyFilter_WithIn_UsesTheKeyColumn() + { + var (db, path) = ConnectAt(t => t.AddTypeRegistration(x => x.Id), strict: true); + using var scoped = db; + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + Partition, + FilterBuilder.Create().Filter(FilterType.In, x => x.Id, new[] { "id-1", "id-3" })); + + results.Select(x => x.Value).OrderBy(x => x, StringComparer.Ordinal).ShouldBe(new[] { "v1", "v3" }); + } + + [TestMethod] + public async Task KeyPropertyFilter_OutsideStrictMode_IsNotRewritten() + { + // Without the write guard the invariant is unenforced, so the ordinary predicate stands. + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: false); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Id, "id-2")); + + results.Select(x => x.Value).ShouldBe(new[] { "v2" }); + } + + [TestMethod] + public async Task KeyPropertyFilter_WithLegacyDivergentRows_FallsBackAndStaysCorrect() + { + // Rows written before the guard existed can violate the invariant. The probe must catch + // that and fall back to the JSON predicate rather than answer from the Key column. + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + + using (var loose = new Tycho(dir, new NewtonsoftJsonSerializer(), dbName: name, rebuildCache: true, requireTypeRegistration: false) + .AddTypeRegistration(x => x.Id) + .Connect()) + { + await loose.WriteObjectsAsync(new[] { new Doc { Id = "id-1", Value = "v1" } }, x => x.Id, Partition); + + // Divergent: stored under a key its id property would never produce. + await loose.WriteObjectsAsync(new[] { new Doc { Id = "id-2", Value = "v2" } }, x => "other", Partition); + } + + SqliteConnection.ClearAllPools(); + + using var strict = new Tycho(dir, new NewtonsoftJsonSerializer(), dbName: name, rebuildCache: false, requireTypeRegistration: true) + .AddTypeRegistration(x => x.Id) + .Connect(); + + var results = + await strict.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Id, "id-2")); + + // Found by content, which the Key column could not have done. + results.Select(x => x.Value).ShouldBe(new[] { "v2" }); + } + + [TestMethod] + public async Task KeyPropertyFilter_CountAndDeleteAgreeWithTheRead() + { + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + await SeedAsync(db); + + var filter = FilterBuilder.Create().Filter(FilterType.In, x => x.Id, new[] { "id-1", "id-3" }); + + (await db.CountObjectsAsync(Partition, filter)).ShouldBe(2); + (await db.DeleteObjectsAsync(Partition, filter)).ShouldBe(2); + (await db.ReadObjectsAsync(Partition)).Select(x => x.Value).ShouldBe(new[] { "v2" }); + } + + [TestMethod] + public async Task KeyPropertyFilter_UnderACamelCasePolicy_StillMatches() + { + // The registered id path and the filter path must both be resolved through the + // serializer, or they would not compare equal and the rewrite would silently not apply. + var serializer = new SystemTextJsonSerializer( + new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true, serializer: serializer); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Id, "id-2")); + + results.Select(x => x.Value).ShouldBe(new[] { "v2" }); + } + + [TestMethod] + public async Task NonKeyPropertyFilter_IsNeverRewritten() + { + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Value, "v2")); + + results.Select(x => x.Id).ShouldBe(new[] { "id-2" }); + } + + [TestMethod] + public async Task NegatedKeyPropertyFilter_StaysOnTheJsonPath() + { + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.NotEquals, x => x.Id, "id-2")); + + results.Select(x => x.Id).OrderBy(x => x, StringComparer.Ordinal).ShouldBe(new[] { "id-1", "id-3" }); + } + + private static async Task PlanFor(Tycho db, string path) + { + await Task.CompletedTask; + db.Dispose(); + SqliteConnection.ClearAllPools(); + + var sb = new StringBuilder(Queries.SelectDataFromJsonValueWithFullTypeName); + var parameters = new FilterParameters(); + var filter = FilterBuilder.Create().Filter(FilterType.Equals, x => x.Id, "id-2"); + filter.Build(sb, new NewtonsoftJsonSerializer(), parameters, new KeyColumnRewrite("$.Id")); + + using var conn = new SqliteConnection($"Data Source={path}"); + conn.Open(); + using var command = conn.CreateCommand(); +#pragma warning disable CA2100 + command.CommandText = "EXPLAIN QUERY PLAN " + sb; +#pragma warning restore CA2100 + command.Parameters.AddWithValue("$fullTypeName", typeof(Doc).FullName); + command.Parameters.AddWithValue("$partition", Partition); + for (var i = 0; i < parameters.Count; i++) + { + command.Parameters.AddWithValue( + FilterParameters.ParameterPrefix + i.ToString(CultureInfo.InvariantCulture), + parameters.Values[i] ?? (object)DBNull.Value); + } + + var plan = new StringBuilder(); + using var reader = command.ExecuteReader(); + while (reader.Read()) + { + plan.AppendLine(reader.GetString(reader.FieldCount - 1)); + } + + return plan.ToString(); + } + + private static async Task SeedAsync(Tycho db) + { + await db.WriteObjectsAsync( + new[] + { + new Doc { Id = "id-1", Value = "v1" }, + new Doc { Id = "id-2", Value = "v2" }, + new Doc { Id = "id-3", Value = "v3" }, + }, + x => x.Id, + Partition); + } + + private static Tycho Connect(Func register, bool strict = false, IJsonSerializer? serializer = null) + => ConnectAt(register, strict, serializer).Db; + + private static (Tycho Db, string Path) ConnectAt(Func register, bool strict = false, IJsonSerializer? serializer = null) + { + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + + var db = new Tycho( + dir, serializer ?? new NewtonsoftJsonSerializer(), dbName: name, rebuildCache: true, requireTypeRegistration: strict); + + return (register(db).Connect(), Path.Combine(dir, name)); + } + + public class Doc + { + public string Id { get; set; } = string.Empty; + + public string Value { get; set; } = string.Empty; + } + + public class Widget + { + public int WidgetId { get; set; } + + public string Value { get; set; } = string.Empty; + } + + public class Both + { + public string Id { get; set; } = string.Empty; + + public string BothId { get; set; } = string.Empty; + } + + public class Keyless + { + public string Value { get; set; } = string.Empty; + } + + public class Indexed + { + public string this[int i] => i.ToString(CultureInfo.InvariantCulture); + + public string Value { get; set; } = string.Empty; + } +} diff --git a/TychoDB.UnitTests/ZzBatchKeyBench.cs b/TychoDB.UnitTests/ZzBatchKeyBench.cs new file mode 100644 index 0000000..1b03ebc --- /dev/null +++ b/TychoDB.UnitTests/ZzBatchKeyBench.cs @@ -0,0 +1,467 @@ +#nullable enable +#pragma warning disable CA1305, CA1307, CA1848, CA2100, SA1600, SA1601, SA1201, SA1202, SA1204, SA1516 + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; +using Microsoft.Data.Sqlite; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace TychoDB.UnitTests; + +/// +/// Scratch harness comparing the ways a batch of keys can be fetched. Not a correctness test — +/// it prints timings. Run explicitly. +/// +[TestClass] +public class ZzBatchKeyBench +{ + private const int RowCount = 250_000; + private const string Partition = "itemMaster|v1"; + + private static readonly IJsonSerializer Serializer = new NewtonsoftJsonSerializer(); + + public class Item + { + public string Key { get; set; } = string.Empty; + + public int DepartmentId { get; set; } + + public string Description { get; set; } = string.Empty; + + public string Filler { get; set; } = string.Empty; + } + + [TestMethod] + [Ignore("Scratch performance harness: seeds 250k rows. Run it by removing this attribute.")] + public async Task Compare() + { + var dir = Path.Combine(Path.GetTempPath(), "tycho_batchkey", Guid.NewGuid().ToString()); + Directory.CreateDirectory(dir); + var name = "bench.db"; + var file = Path.Combine(dir, name); + + var sw = Stopwatch.StartNew(); + using (var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: true, requireTypeRegistration: false).Connect()) + { + const int batch = 10_000; + for (var offset = 0; offset < RowCount; offset += batch) + { + var slice = + Enumerable.Range(offset, Math.Min(batch, RowCount - offset)) + .Select(i => new Item + { + Key = "K" + i.ToString(CultureInfo.InvariantCulture), + DepartmentId = i % 200, + Description = "Item number " + i.ToString(CultureInfo.InvariantCulture), + Filler = new string('x', 200), + }); + + await db.WriteObjectsAsync(slice, x => x.Key, Partition); + } + } + + SqliteConnection.ClearAllPools(); + Console.WriteLine($"seed {RowCount} rows: {sw.ElapsedMilliseconds} ms, {new FileInfo(file).Length / 1_048_576} MB"); + + var rng = new Random(20260828); + var allKeys = Enumerable.Range(0, RowCount).Select(i => "K" + i.ToString(CultureInfo.InvariantCulture)).ToArray(); + + foreach (var batchSize in new[] { 200, 1_000, 5_000, 25_000 }) + { + var keys = Enumerable.Range(0, batchSize).Select(_ => allKeys[rng.Next(RowCount)]).Distinct().ToArray(); + + Console.WriteLine($"\n=== batch of {keys.Length} keys ==="); + + // Tycho paths: end to end, including deserialization. + SqliteConnection.ClearAllPools(); + using (var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: false, requireTypeRegistration: false).Connect()) + { + await Timed("looped ReadObjectAsync", async () => + { + var found = 0; + foreach (var k in keys) + { + if (await db.ReadObjectAsync(k, Partition) is not null) + { + found++; + } + } + + return found; + }); + + await Timed("ReadObjectsByKeysAsync", async () => + (await db.ReadObjectsByKeysAsync(keys, Partition)).Count()); + } + + // Raw SQL shapes: query cost only, no deserialization, so they are not comparable + // with the two above — they are here to compare the shapes against each other. + SqliteConnection.ClearAllPools(); + var conn = Open(file); + try + { + await Timed(" raw json_each(@keys)", () => Task.FromResult(RunJsonEach(conn, keys))); + await Timed(" raw single IN", () => Task.FromResult(RunIn(conn, keys, keys.Length))); + await Timed(" raw chunked IN (900)", () => Task.FromResult(RunIn(conn, keys, 900))); + await Timed(" raw temp table + join", () => Task.FromResult(RunTempTable(conn, keys))); + } + finally + { + conn.Close(); + conn.Dispose(); + SqliteConnection.ClearAllPools(); + } + } + + Directory.Delete(dir, true); + } + + [TestMethod] + [Ignore("Scratch performance harness: seeds 250k rows. Run it by removing this attribute.")] + public async Task CountShapes() + { + var dir = Path.Combine(Path.GetTempPath(), "tycho_countbench", Guid.NewGuid().ToString()); + Directory.CreateDirectory(dir); + var name = "bench.db"; + var file = Path.Combine(dir, name); + + using (var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: true, requireTypeRegistration: false).Connect()) + { + const int batch = 10_000; + for (var offset = 0; offset < RowCount; offset += batch) + { + await db.WriteObjectsAsync( + Enumerable.Range(offset, Math.Min(batch, RowCount - offset)) + .Select(i => new Item + { + Key = "K" + i.ToString(CultureInfo.InvariantCulture), + DepartmentId = i % 200, + Description = "Item number " + i.ToString(CultureInfo.InvariantCulture), + Filler = new string('x', 200), + }), + x => x.Key, + Partition); + } + } + + SqliteConnection.ClearAllPools(); + + using (var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: false, requireTypeRegistration: false).Connect()) + { + await Timed("CountObjectsAsync (all)", async () => await db.CountObjectsAsync(Partition)); + await Timed("CountObjectsAsync (1/200)", async () => await db.CountObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.DepartmentId, 7))); + } + + SqliteConnection.ClearAllPools(); + + var conn = Open(file); + try + { + await Timed(" raw SELECT 1 + client loop", () => Task.FromResult(CountVia(conn, "SELECT 1"))); + await Timed(" raw SELECT COUNT(*)", () => Task.FromResult(CountVia(conn, "SELECT COUNT(*)"))); + } + finally + { + conn.Close(); + conn.Dispose(); + SqliteConnection.ClearAllPools(); + } + + Directory.Delete(dir, true); + } + + /// + /// Two questions at once: what a filter on the key property costs versus reaching the key + /// through the primary key, and what the serializer choice costs on the deserialization + /// that dominates a large read. + /// + [TestMethod] + [Ignore("Scratch performance harness: seeds 250k rows. Run it by removing this attribute.")] + public async Task KeyFilterAndSerializerShapes() + { + foreach (var (serializer, label) in new (IJsonSerializer, string)[] + { + (new NewtonsoftJsonSerializer(), "Newtonsoft"), + (new SystemTextJsonSerializer(), "SystemTextJson"), + }) + { + var dir = Path.Combine(Path.GetTempPath(), "tycho_keybench", Guid.NewGuid().ToString()); + Directory.CreateDirectory(dir); + var name = "bench.db"; + + using (var db = new Tycho(dir, serializer, dbName: name, rebuildCache: true, requireTypeRegistration: false).Connect()) + { + const int batch = 10_000; + for (var offset = 0; offset < RowCount; offset += batch) + { + await db.WriteObjectsAsync( + Enumerable.Range(offset, Math.Min(batch, RowCount - offset)) + .Select(i => new Item + { + Key = "K" + i.ToString(CultureInfo.InvariantCulture), + DepartmentId = i % 200, + Description = "Item number " + i.ToString(CultureInfo.InvariantCulture), + Filler = new string('x', 200), + }), + x => x.Key, + Partition); + } + } + + SqliteConnection.ClearAllPools(); + Console.WriteLine($"\n=== {label} ==="); + + using (var db = new Tycho(dir, serializer, dbName: name, rebuildCache: false, requireTypeRegistration: false).Connect()) + { + // Reaching one row three ways. + await Timed("filter on key property", async () => + (await db.ReadObjectsAsync( + Partition, + FilterBuilder.Create().Filter(FilterType.Equals, x => x.Key, "K123456"))).Count()); + + await Timed("ReadObjectAsync (PK)", async () => + await db.ReadObjectAsync("K123456", Partition) is null ? 0 : 1); + + await Timed("ReadObjectsByKeysAsync (PK)", async () => + (await db.ReadObjectsByKeysAsync(new object[] { "K123456" }, Partition)).Count()); + + // The safe alternative to rewriting the filter onto the Key column: index the + // key property like any other. + await db.CreateIndexAsync(x => x.Key, "ix_key_property"); + + await Timed("filter on key property, indexed", async () => + (await db.ReadObjectsAsync( + Partition, + FilterBuilder.Create().Filter(FilterType.Equals, x => x.Key, "K123456"))).Count()); + + // Deserialization-dominated read: every row in the partition. + await Timed("read all 250k", async () => (await db.ReadObjectsAsync(Partition)).Count()); + } + + SqliteConnection.ClearAllPools(); + Directory.Delete(dir, true); + } + } + + [TestMethod] + [Ignore("Scratch performance harness: seeds 250k rows. Run it by removing this attribute.")] + public async Task KeyColumnRewrite() + { + var dir = Path.Combine(Path.GetTempPath(), "tycho_rewrite", Guid.NewGuid().ToString()); + Directory.CreateDirectory(dir); + var name = "bench.db"; + + using (var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: true, requireTypeRegistration: false).Connect()) + { + const int batch = 10_000; + for (var offset = 0; offset < RowCount; offset += batch) + { + await db.WriteObjectsAsync( + Enumerable.Range(offset, Math.Min(batch, RowCount - offset)) + .Select(i => new Item + { + Key = "K" + i.ToString(CultureInfo.InvariantCulture), + DepartmentId = i % 200, + Description = "Item number " + i.ToString(CultureInfo.InvariantCulture), + Filler = new string('x', 200), + }), + x => x.Key, + Partition); + } + } + + SqliteConnection.ClearAllPools(); + + foreach (var strict in new[] { false, true }) + { + using var db = new Tycho(dir, Serializer, dbName: name, rebuildCache: false, requireTypeRegistration: strict) + .AddTypeRegistration(x => x.Key) + .Connect(); + + Console.WriteLine($"\n=== requireTypeRegistration: {strict} ==="); + + // First call pays the one-time divergence probe (a scan of this type's rows). + var sw = Stopwatch.StartNew(); + _ = (await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Key, "K123456"))).Count(); + sw.Stop(); + Console.WriteLine($" {"first call (incl. probe)",-28} {sw.Elapsed.TotalMilliseconds,9:F1} ms (1 rows)"); + + await Timed("steady state", async () => + (await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Key, "K123456"))).Count()); + + await Timed("In (100 keys)", async () => + (await db.ReadObjectsAsync( + Partition, + FilterBuilder.Create().Filter( + FilterType.In, + x => x.Key, + Enumerable.Range(0, 100).Select(i => "K" + i.ToString(CultureInfo.InvariantCulture))))).Count()); + + SqliteConnection.ClearAllPools(); + } + + Directory.Delete(dir, true); + } + + private static int CountVia(SqliteConnection conn, string projection) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = $"{projection} FROM JsonValue WHERE FullTypeName = $t AND Partition = $p"; + cmd.Parameters.AddWithValue("$t", typeof(Item).FullName!); + cmd.Parameters.AddWithValue("$p", Partition); + + using var reader = cmd.ExecuteReader(); + if (projection == "SELECT COUNT(*)") + { + return reader.Read() ? reader.GetInt32(0) : 0; + } + + var n = 0; + while (reader.Read()) + { + n++; + } + + return n; + } + + private static SqliteConnection Open(string file) + { + var conn = new SqliteConnection($"Data Source={file}"); + conn.Open(); + using var pragma = conn.CreateCommand(); + pragma.CommandText = + "PRAGMA journal_mode = WAL; PRAGMA synchronous = NORMAL; PRAGMA temp_store = MEMORY;" + + "PRAGMA cache_size = -65536; PRAGMA mmap_size = 268435456;"; + pragma.ExecuteNonQuery(); + return conn; + } + + private static async Task Timed(string label, Func> action) + { + // Two warm-up passes (JIT, page cache, statement cache), then the best of five. Best + // rather than mean: the run without a GC pause or a scheduler hiccup is the one that + // reflects the work actually being done. + await action().ConfigureAwait(false); + await action().ConfigureAwait(false); + + var best = double.MaxValue; + var n = 0; + + for (var i = 0; i < 5; i++) + { + var sw = Stopwatch.StartNew(); + n = await action().ConfigureAwait(false); + sw.Stop(); + best = Math.Min(best, sw.Elapsed.TotalMilliseconds); + } + + Console.WriteLine($" {label,-28} {best,9:F1} ms ({n} rows)"); + } + + private static int RunIn(SqliteConnection conn, string[] keys, int chunkSize) + { + var total = 0; + + for (var start = 0; start < keys.Length; start += chunkSize) + { + var end = Math.Min(start + chunkSize, keys.Length); + + var sb = new StringBuilder(); + sb.Append("SELECT Data FROM JsonValue WHERE FullTypeName = $t AND Partition = $p AND Key IN ("); + for (var i = start; i < end; i++) + { + if (i > start) + { + sb.Append(','); + } + + sb.Append("$k").Append(i.ToString(CultureInfo.InvariantCulture)); + } + + sb.Append(')'); + + using var cmd = conn.CreateCommand(); + cmd.CommandText = sb.ToString(); + cmd.Parameters.AddWithValue("$t", typeof(Item).FullName!); + cmd.Parameters.AddWithValue("$p", Partition); + for (var i = start; i < end; i++) + { + cmd.Parameters.AddWithValue("$k" + i.ToString(CultureInfo.InvariantCulture), keys[i]); + } + + total += Drain(cmd); + } + + return total; + } + + private static int RunJsonEach(SqliteConnection conn, string[] keys) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = + "SELECT Data FROM JsonValue WHERE FullTypeName = $t AND Partition = $p" + + " AND Key IN (SELECT value FROM json_each($keys))"; + cmd.Parameters.AddWithValue("$t", typeof(Item).FullName!); + cmd.Parameters.AddWithValue("$p", Partition); + cmd.Parameters.AddWithValue("$keys", JsonSerializer.Serialize(keys)); + + return Drain(cmd); + } + + private static int RunTempTable(SqliteConnection conn, string[] keys) + { + using (var ddl = conn.CreateCommand()) + { + ddl.CommandText = "DROP TABLE IF EXISTS temp.BatchKeys; CREATE TEMP TABLE BatchKeys(Key TEXT PRIMARY KEY);"; + ddl.ExecuteNonQuery(); + } + + using (var tx = conn.BeginTransaction()) + { + using var ins = conn.CreateCommand(); + ins.Transaction = tx; + ins.CommandText = "INSERT OR IGNORE INTO temp.BatchKeys(Key) VALUES ($k)"; + var p = ins.Parameters.Add("$k", SqliteType.Text); + foreach (var k in keys) + { + p.Value = k; + ins.ExecuteNonQuery(); + } + + tx.Commit(); + } + + using var cmd = conn.CreateCommand(); + cmd.CommandText = + "SELECT j.Data FROM temp.BatchKeys b JOIN JsonValue j" + + " ON j.Key = b.Key AND j.FullTypeName = $t AND j.Partition = $p"; + cmd.Parameters.AddWithValue("$t", typeof(Item).FullName!); + cmd.Parameters.AddWithValue("$p", Partition); + + return Drain(cmd); + } + + private static int Drain(SqliteCommand cmd) + { + var n = 0; + using var reader = cmd.ExecuteReader(System.Data.CommandBehavior.SequentialAccess); + while (reader.Read()) + { + _ = reader.GetString(0); + n++; + } + + return n; + } +} diff --git a/TychoDB/Filter.cs b/TychoDB/Filter.cs index 5f793a4..d5e133e 100644 --- a/TychoDB/Filter.cs +++ b/TychoDB/Filter.cs @@ -22,6 +22,18 @@ public enum FilterType GreaterThanOrEqualTo = 6, LessThan = 7, LessThanOrEqualTo = 8, + + /// + /// Matches when the property's value is one of a supplied set. Requires the + /// collection overloads of FilterBuilder<T>.Filter. + /// + In = 9, + + /// + /// Matches when the property's value is none of a supplied set. Requires the + /// collection overloads of FilterBuilder<T>.Filter. + /// + NotIn = 10, } /// diff --git a/TychoDB/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index 9a9ea4d..0faa6c5 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -34,6 +34,27 @@ public class FilterBuilder private const string GreaterThanOrEqual = " >= "; private const string LessThan = " < "; private const string LessThanOrEqual = " <= "; + private const string InOperator = " IN ("; + private const string NotInOperator = " NOT IN ("; + private const string ValueSeparator = ", "; + private const string OrJoin = " OR "; + private const string KeyColumn = "Key"; + private const string AndJoin = " AND "; + + // IN () is a syntax error in SQLite, so an empty set has to be rendered as a + // constant. Dropping the term instead would widen the result set, which is the + // dangerous direction: the caller asked for "none of these" and would get "all". + private const string MatchNothing = "0 = 1"; + private const string MatchEverything = "1 = 1"; + + // Values bind as parameters unless they are genuine numerics or booleans (those + // become literals). SQLITE_MAX_VARIABLE_NUMBER is 32,766 on modern builds but only + // 999 on older ones, and the build in use is the host application's choice, not + // this library's. Longer lists are split into several IN terms joined by OR (AND, + // when negated) rather than capped or rejected, so a large set still works + // everywhere. Lists shorter than this — the overwhelming majority — emit a single + // IN term and are unaffected. + private const int MaxValuesPerInClause = 900; private readonly List _filters = new(); @@ -48,6 +69,8 @@ public static FilterBuilder Create() public FilterBuilder Filter(FilterType filterType, Expression> propertyPath, object value) { + EnsureScalarFilterType(filterType); + // The path is captured as segments rather than rendered here: the serializer that // decides the JSON member names is not known until Build. var propertyPathSegments = QueryPropertyPath.BuildSegments(propertyPath); @@ -62,6 +85,8 @@ public FilterBuilder Filter(FilterType filterType, Expression Filter(FilterType filterType, Expression>> propertyPath, Expression> propertyValuePath, object value) { + EnsureScalarFilterType(filterType); + var propertyPathSegments = QueryPropertyPath.BuildSegments(propertyPath); var propertyValuePathSegments = QueryPropertyPath.BuildSegments(propertyValuePath); var isPropertyValuePathNumeric = QueryPropertyPath.IsNumeric(propertyValuePath); @@ -75,6 +100,8 @@ public FilterBuilder Filter(FilterType filterType, Expre public FilterBuilder Filter(FilterType filterType, string propertyPath, bool isPropertyPathNumeric, bool isPropertyPathBool, bool isPropertyPathDateTime, object value) { + EnsureScalarFilterType(filterType); + // This overload accepts a raw JSON path string from the caller. Because // the path is emitted as an identifier inside JSON_EXTRACT(...) and // cannot be parameterized, validate it against a strict grammar to @@ -86,6 +113,162 @@ public FilterBuilder Filter(FilterType filterType, string propertyPath, bo return this; } + /// + /// Adds a set-membership term: or . + /// + /// A single term rather than a chain of Or()s, so it cannot be mis-grouped, and it + /// renders through the same numeric CAST the scalar comparisons use — which is what + /// lets an expression index over the property serve the query. + /// + /// + /// Duplicate values are removed. An empty set matches nothing for + /// and everything for . A in the set is + /// matched against a missing or null member, which SQL's own IN would never do. + /// + /// + /// The property's type. + /// Must be or . + /// An expression selecting the property to test. + /// The set to test membership against. + /// The current builder for chaining. + public FilterBuilder Filter(FilterType filterType, Expression> propertyPath, IEnumerable? values) + { + // A literal null argument binds here, not to the object overload: IEnumerable is + // the more specific parameter type. Filter(Equals, x => x.Value, null) has always meant + // "compare against null", so it is routed back to the scalar path rather than being + // rejected as a malformed set. + if (values is null) + { + return NullValues(filterType, () => Filter(filterType, propertyPath, (object)null!)); + } + + EnsureSetFilterType(filterType); + + var propertyPathSegments = QueryPropertyPath.BuildSegments(propertyPath); + var isPropertyPathNumeric = QueryPropertyPath.IsNumeric(propertyPath); + var isPropertyPathBool = QueryPropertyPath.IsBool(propertyPath); + var isPropertyPathDateTime = QueryPropertyPath.IsDateTime(propertyPath); + + _filters.Add( + new Filter( + filterType, + null, + propertyPathSegments, + isPropertyPathNumeric, + isPropertyPathBool, + isPropertyPathDateTime, + Distinct(values))); + + return this; + } + + /// + /// Adds a set-membership term against a raw JSON path. See the expression overload for the + /// empty-set, duplicate and null semantics. + /// + /// Must be or . + /// The JSON path to test. + /// Whether the property is numeric. + /// Whether the property is boolean. + /// Whether the property is a date/time. + /// The set to test membership against. + /// The current builder for chaining. + public FilterBuilder Filter(FilterType filterType, string propertyPath, bool isPropertyPathNumeric, bool isPropertyPathBool, bool isPropertyPathDateTime, IEnumerable? values) + { + // See the expression overload: a literal null means the scalar null comparison. + if (values is null) + { + return NullValues( + filterType, + () => Filter(filterType, propertyPath, isPropertyPathNumeric, isPropertyPathBool, isPropertyPathDateTime, (object)null!)); + } + + EnsureSetFilterType(filterType); + QueryPropertyPath.ValidatePath(propertyPath, nameof(propertyPath)); + + _filters.Add( + new Filter( + filterType, + propertyPath, + null, + isPropertyPathNumeric, + isPropertyPathBool, + isPropertyPathDateTime, + Distinct(values))); + + return this; + } + + /// + /// Handles a null passed where a value set was expected: a scalar comparison against null + /// for the ordinary filter types, and an error for In/NotIn, where "no set at all" is not + /// the same thing as the empty set and is far more likely to be an accident. + /// + private FilterBuilder NullValues(FilterType filterType, Func> asScalarNull) + { + if (filterType is FilterType.In or FilterType.NotIn) + { + var empty = filterType == FilterType.In ? "nothing" : "everything"; + var message = $"{filterType} needs a collection; pass an empty one to match {empty}."; + + throw new ArgumentNullException("values", message); + } + + return asScalarNull(); + } + + private static void EnsureSetFilterType(FilterType filterType) + { + if (filterType is not (FilterType.In or FilterType.NotIn)) + { + throw new ArgumentException( + $"{filterType} compares against a single value; pass that value rather than a collection. Only In and NotIn take a collection.", + nameof(filterType)); + } + } + + private static void EnsureScalarFilterType(FilterType filterType) + { + if (filterType is FilterType.In or FilterType.NotIn) + { + throw new ArgumentException( + $"{filterType} tests set membership; use the overload that takes an IEnumerable of values.", + nameof(filterType)); + } + } + + /// + /// Materializes the value set once, dropping duplicates while preserving the caller's + /// order. At most one null survives; the renderer turns it into an IS NULL test. + /// + private static object?[] Distinct(IEnumerable values) + { + var seen = new HashSet(); + var distinct = new List(); + var sawNull = false; + + foreach (var value in values) + { + if (value is null) + { + if (!sawNull) + { + sawNull = true; + distinct.Add(null); + } + + continue; + } + + if (seen.Add(value)) + { + distinct.Add(value); + } + } + + return distinct.ToArray(); + } + public FilterBuilder And() { _filters.Add(new Filter(FilterJoin.And)); @@ -110,13 +293,26 @@ public FilterBuilder EndGroup() return this; } - internal void Build(StringBuilder commandBuilder, IJsonSerializer jsonSerializer, FilterParameters parameters) + internal void Build(StringBuilder commandBuilder, IJsonSerializer jsonSerializer, FilterParameters parameters, KeyColumnRewrite? keyRewrite = null) { - if (_filters.Count > 0) + if (_filters.Count == 0) { - commandBuilder.AppendLine("\nAND"); + return; } + // The caller's filter is only the last conjunct of the generated clause: every query + // this is appended to already reads "WHERE FullTypeName = $fullTypeName AND Partition = + // $partition". SQL binds AND tighter than OR, so emitting the terms bare would let an + // ungrouped Or() split the clause — + // + // (FullTypeName = $t AND Partition = $p AND term1) OR (term2) + // + // — and every term after the first Or() would then be matched against the whole table, + // returning (or, through DeleteObjectsAsync, destroying) rows of other partitions and + // other stored types, which the reader would go on to deserialize as T. The enclosing + // parentheses bind the caller's terms into a single conjunct. + commandBuilder.AppendLine("\nAND").AppendLine(OpenParen); + // Expression-supplied paths were captured as segments; the serializer is only known // here, so render them now against the JSON member names it actually writes. var nameResolver = QueryPropertyPath.AsNameResolver(jsonSerializer); @@ -154,9 +350,16 @@ internal void Build(StringBuilder commandBuilder, IJsonSerializer jsonSerializer } else if (filter.FilterType.HasValue) { + if (keyRewrite is not null && TryBuildKeyColumnFilter(commandBuilder, filter, parameters, keyRewrite)) + { + continue; + } + BuildSimpleFilter(commandBuilder, filter, jsonSerializer, parameters); } } + + commandBuilder.AppendLine(CloseParen); } /// @@ -171,7 +374,7 @@ private static Filter Resolve(in Filter filter, IJsonPropertyNameResolver? nameR return filter; } - var value = ResolveValue(filter.Value, valueResolver); + var value = ResolveSetOrScalarValue(filter, valueResolver); if (filter.PropertyPathSegments is null && filter.PropertyValuePathSegments is null) { @@ -229,6 +432,36 @@ private static Filter Rebuild(in Filter filter, string? propertyPath, string? pr /// as DateOnly/TimeOnly whose ToString() is culture-dependent while /// its JSON form is not. /// + /// + /// Resolves a set-membership term's values element by element, so an enum or DateOnly in an + /// IN list is compared in the same JSON form the serializer wrote — exactly as the scalar + /// comparisons already are. Any other term resolves its single value. + /// + private static object? ResolveSetOrScalarValue(in Filter filter, IJsonValueResolver? valueResolver) + { + if (filter.FilterType is not (FilterType.In or FilterType.NotIn) || filter.Value is not object?[] values) + { + return ResolveValue(filter.Value, valueResolver); + } + + object?[]? resolved = null; + + for (var i = 0; i < values.Length; i++) + { + var value = ResolveValue(values[i], valueResolver); + + if (resolved is null && ReferenceEquals(value, values[i])) + { + continue; + } + + resolved ??= (object?[])values.Clone(); + resolved[i] = value; + } + + return resolved ?? values; + } + private static object? ResolveValue(object? value, IJsonValueResolver? valueResolver) { if (valueResolver is null || value is null) @@ -483,6 +716,77 @@ private void BuildExistsFilter(StringBuilder commandBuilder, in Filter filter, I } } + /// + /// Emits an equality or set-membership test against the Key column when the filter + /// targets the type's id property, and reports whether it did. + /// + /// Only and are rewritten. Their + /// negations would still scan — a negated predicate cannot use the index either way — so + /// rewriting them would trade a clear predicate for no gain. A null comparison value is left + /// alone too: Key is NOT NULL, so "the id is null" is a question about the + /// document, not about the key. + /// + /// + /// Values bind as text because that is the form the key was stored in — both the write path + /// and the by-key reads put the key through ToString(). + /// + /// + private static bool TryBuildKeyColumnFilter(StringBuilder commandBuilder, in Filter filter, FilterParameters parameters, KeyColumnRewrite keyRewrite) + { + if (!string.Equals(filter.PropertyPath, keyRewrite.ResolvedIdPath, StringComparison.Ordinal) + || !string.IsNullOrEmpty(filter.PropertyValuePath)) + { + return false; + } + + switch (filter.FilterType!.Value) + { + case FilterType.Equals when filter.Value is not null: + commandBuilder.Append(KeyColumn).Append(Equals) + .Append(parameters.Add(filter.Value.ToString())) + .AppendLine(); + return true; + + case FilterType.In when filter.Value is object?[] values && Array.IndexOf(values, null) < 0: + if (values.Length == 0) + { + commandBuilder.AppendLine(MatchNothing); + return true; + } + + for (var chunk = 0; chunk * MaxValuesPerInClause < values.Length; chunk++) + { + if (chunk > 0) + { + commandBuilder.Append(OrJoin); + } + + var start = chunk * MaxValuesPerInClause; + var end = Math.Min(start + MaxValuesPerInClause, values.Length); + + commandBuilder.Append(KeyColumn).Append(InOperator); + + for (var i = start; i < end; i++) + { + if (i > start) + { + commandBuilder.Append(ValueSeparator); + } + + commandBuilder.Append(parameters.Add(values[i]!.ToString())); + } + + commandBuilder.Append(CloseParen); + } + + commandBuilder.AppendLine(); + return true; + + default: + return false; + } + } + private void BuildSimpleFilter(StringBuilder commandBuilder, in Filter filter, IJsonSerializer jsonSerializer, FilterParameters parameters) { switch (filter.FilterType!.Value) @@ -586,6 +890,135 @@ private void BuildSimpleFilter(StringBuilder commandBuilder, in Filter filter, I AppendLike(commandBuilder, parameters, filter.Value, leadingWildcard: false, trailingWildcard: true); commandBuilder.AppendLine(); break; + + case FilterType.In: + case FilterType.NotIn: + BuildSetFilter(commandBuilder, filter, jsonSerializer, parameters); + break; + } + } + + /// + /// Renders path IN (…) / path NOT IN (…). The path is emitted through the same + /// helpers the scalar comparisons use, so a numeric property keeps its + /// CAST(… as NUMERIC) form and stays matchable by an expression index over it. + /// + private static void BuildSetFilter(StringBuilder commandBuilder, in Filter filter, IJsonSerializer jsonSerializer, FilterParameters parameters) + { + var negated = filter.FilterType!.Value == FilterType.NotIn; + var values = filter.Value as object?[] ?? Array.Empty(); + + // SQL's IN never matches NULL against a NULL in the list, so a null the caller put in + // the set is pulled out and tested separately. Without this it would silently be a + // value that can never match. + var hasNull = false; + var present = new List(values.Length); + + foreach (var value in values) + { + if (value is null) + { + hasNull = true; + } + else + { + present.Add(value); + } + } + + var join = negated ? AndJoin : OrJoin; + + if (present.Count == 0) + { + if (hasNull) + { + AppendSetPath(commandBuilder, filter); + commandBuilder.Append(negated ? IsNotNull : IsNull).AppendLine(); + } + else + { + commandBuilder.AppendLine(negated ? MatchEverything : MatchNothing); + } + + return; + } + + var chunks = ((present.Count - 1) / MaxValuesPerInClause) + 1; + var wrap = hasNull || chunks > 1; + + if (wrap) + { + commandBuilder.Append(OpenParen); + } + + for (var chunk = 0; chunk < chunks; chunk++) + { + if (chunk > 0) + { + commandBuilder.Append(join); + } + + var start = chunk * MaxValuesPerInClause; + var end = Math.Min(start + MaxValuesPerInClause, present.Count); + + AppendSetPath(commandBuilder, filter); + commandBuilder.Append(negated ? NotInOperator : InOperator); + + for (var i = start; i < end; i++) + { + if (i > start) + { + commandBuilder.Append(ValueSeparator); + } + + AppendSetValue(commandBuilder, filter, parameters, present[i], jsonSerializer); + } + + commandBuilder.Append(CloseParen); + } + + if (hasNull) + { + commandBuilder.Append(join); + AppendSetPath(commandBuilder, filter); + commandBuilder.Append(negated ? IsNotNull : IsNull); + } + + if (wrap) + { + commandBuilder.Append(CloseParen); + } + + commandBuilder.AppendLine(); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void AppendSetPath(StringBuilder commandBuilder, in Filter filter) + { + if (filter.IsPropertyPathNumeric) + { + AppendCastNumeric(commandBuilder, filter.PropertyPath!); + } + else + { + AppendJsonExtract(commandBuilder, filter.PropertyPath!); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void AppendSetValue(StringBuilder commandBuilder, in Filter filter, FilterParameters parameters, object? value, IJsonSerializer jsonSerializer) + { + if (filter.IsPropertyPathNumeric) + { + AppendNumericValue(commandBuilder, parameters, value); + } + else if (filter.IsPropertyPathDateTime) + { + commandBuilder.Append(parameters.Add(GetDateTimeString(value, jsonSerializer))); + } + else + { + AppendValue(commandBuilder, parameters, value); } } diff --git a/TychoDB/KeyColumnRewrite.cs b/TychoDB/KeyColumnRewrite.cs new file mode 100644 index 0000000..a9e9f30 --- /dev/null +++ b/TychoDB/KeyColumnRewrite.cs @@ -0,0 +1,71 @@ +using System; +using System.Collections.Concurrent; +using Microsoft.Data.Sqlite; + +namespace TychoDB; + +/// +/// Lets an equality or set-membership filter on a type's id property be answered from the +/// indexed Key column instead of a JSON_EXTRACT scan of every row. +/// +/// +/// +/// The rewrite is only correct while every row of the type is keyed by that id property, which +/// Tycho does not guarantee in general: WriteObjectsAsync(objs, keySelector, …) takes a +/// key at the call site and may disagree with the registration. Two things together make it +/// safe here. Strict registration rejects a divergent write outright, so no row written through +/// this instance can break the invariant; and rows already in the database — written by an +/// earlier version, or outside strict mode — are checked once with +/// before the rewrite is used for that +/// type. A single divergent row disables the rewrite for the type, and the ordinary predicate +/// is emitted instead, so the worst case is the performance that was there before. +/// +/// +/// The probe is a scan, so it is run lazily — only when a query that could benefit actually +/// arrives — and its verdict is cached for the lifetime of the connection. +/// +/// +internal sealed class KeyColumnRewrite +{ + private readonly ConcurrentDictionary _usableByTypeName = new(StringComparer.Ordinal); + + public KeyColumnRewrite(string resolvedIdPath) + { + ResolvedIdPath = resolvedIdPath; + } + + /// + /// Gets the id property's JSON path, already rendered through the serializer's member names + /// so it can be compared directly against a filter's resolved path. + /// + public string ResolvedIdPath { get; } + + /// + /// Returns this rewrite when the stored keys for provably + /// match the id property, and when they do not — in which case the + /// caller emits the ordinary JSON path predicate. + /// + public KeyColumnRewrite? VerifiedFor(SqliteConnection connection, string fullTypeName) + { + var usable = + _usableByTypeName.GetOrAdd( + fullTypeName, + static (name, state) => !state.Self.HasDivergentRow(state.Connection, name), + (Self: this, Connection: connection)); + + return usable ? this : null; + } + + private bool HasDivergentRow(SqliteConnection connection, string fullTypeName) + { + using var command = connection.CreateCommand(); + +#pragma warning disable CA2100 // The path is rendered from a property expression and validated by QueryPropertyPath. + command.CommandText = Queries.SelectKeyDivergesFromIdProperty(ResolvedIdPath); +#pragma warning restore CA2100 + command.Parameters.Add("$fullTypeName", SqliteType.Text).Value = fullTypeName; + + using var reader = command.ExecuteReader(); + return reader.Read(); + } +} diff --git a/TychoDB/Queries.cs b/TychoDB/Queries.cs index e752f9d..d3a7c79 100644 --- a/TychoDB/Queries.cs +++ b/TychoDB/Queries.cs @@ -219,6 +219,29 @@ FROM StreamValue LIMIT 1 """; + // Divergence probe for the key-column rewrite. Answers "is every row of this type keyed by + // its id property?" — the invariant that lets a filter on the id property be answered from + // the indexed Key column instead of a JSON_EXTRACT scan. A row whose id member is missing + // counts as divergent: its key cannot have come from a value that is not there. Returns at + // most one row, and only rows written before the strict-mode write guard existed (or + // outside strict mode) can produce it. + public static string SelectKeyDivergesFromIdProperty(string resolvedIdPath) + { + return string.Concat( + """ + SELECT 1 + FROM JsonValue + Where + FullTypeName = $fullTypeName + AND + (JSON_EXTRACT(Data, ' + """.TrimEnd(), + resolvedIdPath, + "') IS NULL OR Key <> CAST(JSON_EXTRACT(Data, '", + resolvedIdPath, + "') AS TEXT))\nLIMIT 1"); + } + public const string SelectPartitions = """ SELECT DISTINCT Partition @@ -235,14 +258,49 @@ FROM JsonValue Partition = $partition """; + // COUNT(*), not "SELECT 1" counted row by row on the client: the engine counts without + // materializing a result row per match, and the reader makes one round trip instead of one + // per matching row. Measured 2.3x faster counting a 250,000-row partition (5.0 ms vs + // 11.6 ms). This also halves the pre-count a progress-reporting read performs. public const string SelectCountFromJsonValueWithFullTypeName = """ - SELECT 1 + SELECT COUNT(*) + FROM JsonValue + Where + FullTypeName = $fullTypeName + AND + Partition = $partition + """; + + // Batch key lookup. The key set arrives as a single JSON array bound to $keys and is + // expanded by JSON_EACH, rather than as one bound parameter per key. That keeps the + // statement text and parameter count constant no matter how many keys are asked for, + // so it is not subject to SQLITE_MAX_VARIABLE_NUMBER (999 on older builds) and does not + // force a distinct statement — and therefore a re-parse and re-plan — for every batch + // size. Key leads the PRIMARY KEY (Key, FullTypeName, Partition), so each expanded key + // is a primary-key probe. JSON1 is verified at connect, so JSON_EACH is always present. + public const string SelectDataFromJsonValueWithFullTypeNameAndKeys = + """ + SELECT rowid, Data + FROM JsonValue + Where + FullTypeName = $fullTypeName + AND + Partition = $partition + AND + Key IN (SELECT value FROM JSON_EACH($keys)) + """; + + public const string SelectCountFromJsonValueWithFullTypeNameAndKeys = + """ + SELECT COUNT(*) FROM JsonValue Where FullTypeName = $fullTypeName AND Partition = $partition + AND + Key IN (SELECT value FROM JSON_EACH($keys)) """; public const string DeleteDataFromJsonValueWithKeyAndFullTypeName = diff --git a/TychoDB/RegisteredTypeInformation.cs b/TychoDB/RegisteredTypeInformation.cs index dcc7904..292a753 100644 --- a/TychoDB/RegisteredTypeInformation.cs +++ b/TychoDB/RegisteredTypeInformation.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq.Expressions; +using System.Reflection; namespace TychoDB; @@ -54,6 +55,13 @@ public RegisteredTypeInformation(bool requiresIdMapping, string? typeFullName, s public string? IdPropertyPath { get; private set; } + /// + /// Gets the id property captured as unresolved segments, so it can be rendered against the + /// serializer's JSON member names at query time exactly as a filter path is. Null unless the + /// type was registered by property expression. + /// + internal PropertyPathSegment[]? IdPropertyPathSegments { get; private set; } + public bool IsNumeric { get; private set; } public bool IsBool { get; private set; } @@ -130,7 +138,10 @@ x1 is TId id1 && x2 is TId id2 && new RegisteredTypeInformation(requiresIdMapping: false, idSelector: compiledExpression!, idComparer: idComparerFunc!, idProperty: idProperty.GetExpressionMemberName(), idPropertyPath: QueryPropertyPath.BuildPath(idProperty), isNumeric: QueryPropertyPath.IsNumeric(idProperty), isBool: QueryPropertyPath.IsBool(idProperty), typeFullName: type.FullName, - typeName: type.Name, safeTypeName: type.GetSafeTypeName(), typeNamespace: type.Namespace, objectType: type!); + typeName: type.Name, safeTypeName: type.GetSafeTypeName(), typeNamespace: type.Namespace, objectType: type!) + { + IdPropertyPathSegments = QueryPropertyPath.BuildSegments(idProperty), + }; return rti; } @@ -154,12 +165,87 @@ x1 is string id1 && x2 is string id2 && typeName: type.Name, safeTypeName: type.GetSafeTypeName(), typeNamespace: type.Namespace, objectType: type!); } + /// + /// Registers , detecting its id property by convention. + /// + /// The property is looked for by name, in order: Id, then <TypeName>Id + /// (for example PersonId on Person), each matched case-insensitively. It must + /// be a public, readable, non-indexed instance property. + /// + /// + /// When no such property exists the type is still registered — satisfying + /// requireTypeRegistration — but without an id mapping, so it can only be reached by + /// an explicitly supplied key, exactly as before. That keeps registering a key-less type + /// and writing it with a call-site key selector working. + /// + /// + /// The type to register. + /// The registration information for . public static RegisteredTypeInformation Create() { var type = typeof(T); + var idProperty = FindConventionalIdProperty(type); + + if (idProperty is not null) + { + return Create(BuildPropertySelector(idProperty)); + } + return new RegisteredTypeInformation(requiresIdMapping: true, typeFullName: type.FullName, typeName: type.Name, safeTypeName: type.GetSafeTypeName(), typeNamespace: type.Namespace, objectType: type!); } + + /// + /// Finds the property a conventional id would live on: Id, else + /// <TypeName>Id. Returns null when neither exists. + /// + private static PropertyInfo? FindConventionalIdProperty(Type type) + { + var properties = + type.GetProperties(BindingFlags.Public | BindingFlags.Instance); + + // "Id" wins over "Id" when a type declares both, so the more specific name + // never quietly shadows the obvious one. + foreach (var candidate in new[] { "Id", type.Name + "Id" }) + { + foreach (var property in properties) + { + if (!property.Name.Equals(candidate, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + // An id has to be readable and addressable as a plain member: an indexer or a + // write-only property cannot be turned into a property path. + if (property.GetIndexParameters().Length > 0 || !property.CanRead) + { + continue; + } + + return property; + } + } + + return null; + } + + /// + /// Builds the x => x.Prop expression the property-based registration path expects, + /// boxing value types through a Convert node exactly as a hand-written + /// Expression<Func<T, object>> would. + /// + private static Expression> BuildPropertySelector(PropertyInfo property) + { + var parameter = Expression.Parameter(typeof(T), "x"); + Expression body = Expression.Property(parameter, property); + + if (property.PropertyType.IsValueType) + { + body = Expression.Convert(body, typeof(object)); + } + + return Expression.Lambda>(body, parameter); + } } diff --git a/TychoDB/Tycho.cs b/TychoDB/Tycho.cs index c866ec9..d012f02 100644 --- a/TychoDB/Tycho.cs +++ b/TychoDB/Tycho.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Linq.Expressions; using System.Text; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.Data.Sqlite; @@ -21,6 +22,7 @@ private const string ParameterFullTypeName = "$fullTypeName", ParameterPartition = "$partition", ParameterKey = "$key", + ParameterKeys = "$keys", ParameterJson = "$json", ParameterBlob = "$blob", ParameterBlobLength = "$blobLength", @@ -45,6 +47,9 @@ private const string private readonly IJsonSerializer _jsonSerializer; private readonly bool _persistConnection; private readonly bool _requireTypeRegistration; + + // One rewrite per type, so its divergence verdict is probed once and reused. + private readonly ConcurrentDictionary _keyColumnRewrites = new(); private readonly int _commandTimeout; private readonly Dictionary _registeredTypeInformation = new(); @@ -330,6 +335,20 @@ public ValueTask WriteObjectAsync(T obj, string? partition = null, bool /// Whether to use a transaction for the operation. /// A token to cancel the asynchronous operation. /// A ValueTask containing a boolean indicating success or failure. + /// + /// The key this selector returns is the key the row is stored under, and it overrides any + /// key the type's registration would supply. If the two disagree, the by-object overloads + /// keep using the registered key and stop finding the row: + /// + /// returns null and DeleteObjectAsync(obj) returns false without deleting anything, + /// while the row is still there under the key this selector produced. + /// + /// Under requireTypeRegistration, a type registered by id property will not let that + /// happen: a selector that disagrees with the registration throws + /// rather than writing a row the by-object overloads cannot reach. Outside strict mode the + /// override is permitted and unchecked. + /// + /// public ValueTask WriteObjectAsync(T obj, Func keySelector, string? partition = null, bool withTransaction = true, CancellationToken cancellationToken = default) { @@ -361,6 +380,20 @@ public ValueTask WriteObjectsAsync(IEnumerable objs, string? partiti /// Whether to use a transaction for the operation. /// A token to cancel the asynchronous operation. /// A ValueTask containing a boolean indicating success or failure. + /// + /// The key this selector returns is the key the row is stored under, and it overrides any + /// key the type's registration would supply. If the two disagree, the by-object overloads + /// keep using the registered key and stop finding the row: + /// + /// returns null and DeleteObjectAsync(obj) returns false without deleting anything, + /// while the row is still there under the key this selector produced. + /// + /// Under requireTypeRegistration, a type registered by id property will not let that + /// happen: a selector that disagrees with the registration throws + /// rather than writing a row the by-object overloads cannot reach. Outside strict mode the + /// override is permitted and unchecked. + /// + /// public ValueTask WriteObjectsAsync(IEnumerable objs, Func keySelector, string? partition = null, bool withTransaction = true, CancellationToken cancellationToken = default) { @@ -368,6 +401,8 @@ public ValueTask WriteObjectsAsync(IEnumerable objs, Func ArgumentNullException.ThrowIfNull(keySelector); ArgumentNullException.ThrowIfNull(_connection); + keySelector = GuardAgainstKeyDivergence(keySelector); + return _connection .WithConnectionBlockAsync( _connectionGate, @@ -525,7 +560,7 @@ public ValueTask CountObjectsAsync(string? partition = null, FilterBuild return _connection .WithConnectionBlockAsync( _connectionGate, - (partition, filter, withTransaction, commandBuilder: _commandBuilder, _jsonSerializer), + (partition, filter, withTransaction, commandBuilder: _commandBuilder, _jsonSerializer, keyRewrite: GetKeyColumnRewriteFor()), static (conn, state) => { SqliteTransaction? transaction = null; @@ -548,7 +583,11 @@ public ValueTask CountObjectsAsync(string? partition = null, FilterBuild var filterParameters = new FilterParameters(); if (state.filter is not null) { - state.filter.Build(state.commandBuilder, state._jsonSerializer, filterParameters); + state.filter.Build( + state.commandBuilder, + state._jsonSerializer, + filterParameters, + state.keyRewrite?.VerifiedFor(conn, TypeCache.FullName)); } #pragma warning disable CA2100 // Comparison values are parameterized (AddFilterParameters); only validated JSON paths/identifiers are concatenated. @@ -558,12 +597,8 @@ public ValueTask CountObjectsAsync(string? partition = null, FilterBuild using var reader = selectCommand.ExecuteReader(); - int count = 0; - - while (reader.Read()) - { - ++count; - } + // One row holding the count, rather than one row per match. + int count = reader.Read() ? reader.GetInt32(0) : 0; transaction?.Commit(); @@ -848,6 +883,96 @@ public ValueTask> ReadObjectsAsync( bool withTransaction = false, IProgress? progress = null, CancellationToken cancellationToken = default) + => ReadObjectsCoreAsync(partition, filter, sort, top, withTransaction, progress, null, cancellationToken); + + /// + /// Reads the objects stored under a set of keys, in one round trip. + /// + /// Keys lead the primary key, so each is a primary-key probe; a filter on the key + /// property goes through JSON_EXTRACT instead and scans. Prefer this over a + /// loop of : + /// it takes the connection gate once rather than once per key, which matters under + /// contention, and it measured 2.1–2.7x faster end to end than the loop across batches of + /// 200 to 24,000 keys on a 250,000-row store. Both include deserialization, which is + /// identical between them and dominates the remainder; the query alone is roughly 2.5x + /// faster again. + /// + /// + /// Keys are bound as a single JSON array, so there is no limit on how many may be passed + /// and no chunking to think about. Keys that are not present are simply absent from the + /// result, so the result may be shorter than the key set, and its order is the database's, + /// not the key set's. Duplicate keys yield one object each. + /// + /// + /// The type of objects to read. + /// The keys to read. An empty set returns no objects without querying. + /// Optional partition to read from. + /// Optional sorting to apply to the result set. + /// Whether to use a transaction for the operation. + /// Optional progress reporter; see . + /// A token to cancel the asynchronous operation. + /// A ValueTask containing the objects found for those keys. + public ValueTask> ReadObjectsByKeysAsync( + IEnumerable keys, + string? partition = null, + SortBuilder? sort = null, + bool withTransaction = false, + IProgress? progress = null, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(keys); + + var keysJson = BuildKeyArrayJson(keys); + + if (keysJson is null) + { + return new ValueTask>(Array.Empty()); + } + + return ReadObjectsCoreAsync(partition, null, sort, null, withTransaction, progress, keysJson, cancellationToken); + } + + /// + /// Renders the key set as a JSON array of strings for JSON_EACH to expand, using the same + /// ToString() form the single-key overloads bind. Returns null for an empty set, which has + /// no query to run. + /// + private static string? BuildKeyArrayJson(IEnumerable keys) + { + using var buffer = new MemoryStream(); + using (var writer = new Utf8JsonWriter(buffer)) + { + writer.WriteStartArray(); + + var any = false; + foreach (var key in keys) + { + ArgumentNullException.ThrowIfNull(key, nameof(keys)); + + writer.WriteStringValue(key.ToString()); + any = true; + } + + if (!any) + { + return null; + } + + writer.WriteEndArray(); + } + + return Encoding.UTF8.GetString(buffer.GetBuffer(), 0, (int)buffer.Length); + } + + private ValueTask> ReadObjectsCoreAsync( + string? partition, + FilterBuilder? filter, + SortBuilder? sort, + int? top, + bool withTransaction, + IProgress? progress, + string? keysJson, + CancellationToken cancellationToken) { if (_requireTypeRegistration) { @@ -857,9 +982,9 @@ public ValueTask> ReadObjectsAsync( ArgumentNullException.ThrowIfNull(_connection); return _connection - .WithConnectionBlockAsync, (string? partition, FilterBuilder? filter, SortBuilder? sort, int? top, bool withTransaction, IProgress? progress, StringBuilder commandBuilder, int commandTimeout, IJsonSerializer jsonSerializer, CancellationToken cancellationToken)>( + .WithConnectionBlockAsync, (string? partition, FilterBuilder? filter, SortBuilder? sort, int? top, bool withTransaction, IProgress? progress, StringBuilder commandBuilder, int commandTimeout, IJsonSerializer jsonSerializer, string? keysJson, KeyColumnRewrite? keyRewrite, CancellationToken cancellationToken)>( _connectionGate, - (partition, filter, sort, top, withTransaction, progress, _commandBuilder, _commandTimeout, _jsonSerializer, cancellationToken), + (partition, filter, sort, top, withTransaction, progress, _commandBuilder, _commandTimeout, _jsonSerializer, keysJson, GetKeyColumnRewriteFor(), cancellationToken), static async (conn, state) => { SqliteTransaction? transaction = null; @@ -870,13 +995,21 @@ static async (conn, state) => } var commandBuilder = state.commandBuilder; - commandBuilder.Clear().Append(Queries.SelectDataFromJsonValueWithFullTypeName); + commandBuilder.Clear().Append( + state.keysJson is null + ? Queries.SelectDataFromJsonValueWithFullTypeName + : Queries.SelectDataFromJsonValueWithFullTypeNameAndKeys); // Apply filters and sorting var filterParameters = new FilterParameters(); + + // Resolved once and reused by the progress pre-count below, so the count and + // the rows it is measuring can never be built from different predicates. + var keyRewrite = state.keyRewrite?.VerifiedFor(conn, TypeCache.FullName); + if (state.filter is not null) { - state.filter.Build(commandBuilder, state.jsonSerializer, filterParameters); + state.filter.Build(commandBuilder, state.jsonSerializer, filterParameters, keyRewrite); } if (state.sort is not null) @@ -904,6 +1037,11 @@ static async (conn, state) => // Use cached parameters selectCommand.Parameters.Add(new SqliteParameter(ParameterFullTypeName, SqliteType.Text) { Value = TypeCache.FullName }); selectCommand.Parameters.Add(new SqliteParameter(ParameterPartition, SqliteType.Text) { Value = state.partition.AsValueOrEmptyString() }); + if (state.keysJson is not null) + { + selectCommand.Parameters.Add(new SqliteParameter(ParameterKeys, SqliteType.Text) { Value = state.keysJson }); + } + selectCommand.AddFilterParameters(filterParameters); // Overall progress needs the result-set size up front. Count with the same @@ -914,12 +1052,15 @@ static async (conn, state) => if (state.progress is not null) { - commandBuilder.Clear().Append(Queries.SelectCountFromJsonValueWithFullTypeName); + commandBuilder.Clear().Append( + state.keysJson is null + ? Queries.SelectCountFromJsonValueWithFullTypeName + : Queries.SelectCountFromJsonValueWithFullTypeNameAndKeys); var countFilterParameters = new FilterParameters(); if (state.filter is not null) { - state.filter.Build(commandBuilder, state.jsonSerializer, countFilterParameters); + state.filter.Build(commandBuilder, state.jsonSerializer, countFilterParameters, keyRewrite); } using var countCommand = conn.CreateCommand(); @@ -930,12 +1071,17 @@ static async (conn, state) => countCommand.CommandTimeout = state.commandTimeout; countCommand.Parameters.Add(new SqliteParameter(ParameterFullTypeName, SqliteType.Text) { Value = TypeCache.FullName }); countCommand.Parameters.Add(new SqliteParameter(ParameterPartition, SqliteType.Text) { Value = state.partition.AsValueOrEmptyString() }); + if (state.keysJson is not null) + { + countCommand.Parameters.Add(new SqliteParameter(ParameterKeys, SqliteType.Text) { Value = state.keysJson }); + } + countCommand.AddFilterParameters(countFilterParameters); await using var countReader = await countCommand.ExecuteReaderAsync(state.cancellationToken).ConfigureAwait(false); - while (countReader.Read()) + if (countReader.Read()) { - ++totalRows; + totalRows = countReader.GetInt64(0); } if (state.top is not null && totalRows > state.top.Value) @@ -1123,9 +1269,9 @@ await ReadObjectsWithKeysAsync(innerObjectSelection, partition, filter, withTran string selectionPath = QueryPropertyPath.BuildPath(innerObjectSelection, NameResolver); return _connection - .WithConnectionBlockAsync, (string selectionPath, string? partition, FilterBuilder? filter, bool withTransaction, StringBuilder commandBuilder, IJsonSerializer jsonSerializer, CancellationToken cancellationToken)>( + .WithConnectionBlockAsync, (string selectionPath, string? partition, FilterBuilder? filter, bool withTransaction, StringBuilder commandBuilder, IJsonSerializer jsonSerializer, KeyColumnRewrite? keyRewrite, CancellationToken cancellationToken)>( _connectionGate, - (selectionPath, partition, filter, withTransaction, _commandBuilder, _jsonSerializer, cancellationToken), + (selectionPath, partition, filter, withTransaction, _commandBuilder, _jsonSerializer, keyRewrite: GetKeyColumnRewriteFor(), cancellationToken), static async (conn, state) => { SqliteTransaction? transaction = null; @@ -1152,7 +1298,11 @@ static async (conn, state) => var filterParameters = new FilterParameters(); if (state.filter is not null) { - state.filter.Build(commandBuilder, state.jsonSerializer, filterParameters); + state.filter.Build( + commandBuilder, + state.jsonSerializer, + filterParameters, + state.keyRewrite?.VerifiedFor(conn, typeof(TIn).FullName!)); } #pragma warning disable CA2100 // Comparison values are parameterized (AddFilterParameters); only validated JSON paths/identifiers are concatenated. @@ -1307,7 +1457,7 @@ public ValueTask DeleteObjectsAsync(string? partition = null, FilterBuil return _connection .WithConnectionBlockAsync( _connectionGate, - (partition, filter, withTransaction, commandBuilder: _commandBuilder, _jsonSerializer), + (partition, filter, withTransaction, commandBuilder: _commandBuilder, _jsonSerializer, keyRewrite: GetKeyColumnRewriteFor()), static (conn, state) => { SqliteTransaction? transaction = null; @@ -1330,7 +1480,11 @@ public ValueTask DeleteObjectsAsync(string? partition = null, FilterBuil var filterParameters = new FilterParameters(); if (state.filter is not null) { - state.filter.Build(state.commandBuilder, state._jsonSerializer, filterParameters); + state.filter.Build( + state.commandBuilder, + state._jsonSerializer, + filterParameters, + state.keyRewrite?.VerifiedFor(conn, TypeCache.FullName)); } #pragma warning disable CA2100 // Comparison values are parameterized (AddFilterParameters); only validated JSON paths/identifiers are concatenated. @@ -2398,6 +2552,88 @@ public Func GetIdSelectorFor() return rti.GetIdSelector(); } + /// + /// Builds the key-column rewrite candidate for , or null when the + /// preconditions do not hold: strict registration must be on (so the write guard forbids a + /// divergent key) and the type must have been registered by id property (so there is a path + /// to compare a filter against). The candidate still has to clear its divergence probe + /// against the data before it is used. + /// + private KeyColumnRewrite? GetKeyColumnRewriteFor() + { + if (!_requireTypeRegistration + || !_registeredTypeInformation.TryGetValue(typeof(T), out var rti) + || rti is null + || rti.RequiresIdMapping + || rti.IdPropertyPathSegments is null) + { + return null; + } + + return _keyColumnRewrites.GetOrAdd( + typeof(T), + static (_, state) => + new KeyColumnRewrite( + QueryPropertyPath.RenderPath( + state.Segments, + QueryPropertyPath.AsNameResolver(state.Serializer))), + (Segments: rti.IdPropertyPathSegments, Serializer: _jsonSerializer)); + } + + /// + /// In strict mode, wraps a caller-supplied key selector so that a key disagreeing with the + /// type's registered id property is rejected instead of written. + /// + /// A row stored under a key the registration would not produce is unreachable by + /// ReadObjectAsync(obj), ObjectExistsAsync(obj) and DeleteObjectAsync(obj), + /// all of which key off the registration — and the delete failure is silent, returning false + /// while the row survives. Turning that into an exception at the write is the only point + /// where the disagreement is still visible. + /// + /// + /// The check is a wrapper rather than a pre-pass so the object sequence is enumerated once: + /// callers routinely pass a lazy query. It applies only when strict registration is on and + /// the type was registered by id property — a delegate registration has no property to + /// compare against, and outside strict mode the override is deliberate and permitted. + /// + /// + private Func GuardAgainstKeyDivergence(Func keySelector) + { + if (!_requireTypeRegistration + || !_registeredTypeInformation.TryGetValue(typeof(T), out var rti) + || rti is null + || rti.RequiresIdMapping + || rti.IdPropertyPath is null) + { + return keySelector; + } + + var registeredSelector = rti.GetIdSelector(); + var idProperty = rti.IdProperty; + + return obj => + { + var supplied = keySelector(obj); + + // Compared as text because that is what the Key column stores: both sides go + // through ToString() on their way into the database. + var suppliedKey = supplied?.ToString(); + var registeredKey = registeredSelector(obj)?.ToString(); + + if (!string.Equals(suppliedKey, registeredKey, StringComparison.Ordinal)) + { + throw new TychoException( + $"The supplied key selector produced \"{suppliedKey}\" for {typeof(T).Name}, but its registered id property {idProperty} gives \"{registeredKey}\". " + + $"A row written under \"{suppliedKey}\" could not be read or deleted by object, because those overloads use the registered id. " + + "Supply the registered key, register the type with a custom key selector instead, or turn off requireTypeRegistration."); + } + + // Func promises a non-null key; the null-conditional above is defensive + // against a selector that breaks that contract, not an admission that it may. + return supplied!; + }; + } + /// /// Gets the ID value for an object instance. /// diff --git a/TychoDB/TychoQueryable.cs b/TychoDB/TychoQueryable.cs index a8c35e9..00865d4 100644 --- a/TychoDB/TychoQueryable.cs +++ b/TychoDB/TychoQueryable.cs @@ -271,15 +271,11 @@ private FilterBuilder BuildFilterFromExpressionInternal(Expression expression { case ExpressionType.AndAlso: // x => x.A && x.B - filterBuilder = BuildFilterFromExpressionInternal(binaryExpression.Left, filterBuilder); - filterBuilder = filterBuilder.And(); - return BuildFilterFromExpressionInternal(binaryExpression.Right, filterBuilder); + return BuildBooleanExpression(binaryExpression, filterBuilder, or: false); case ExpressionType.OrElse: // x => x.A || x.B - filterBuilder = BuildFilterFromExpressionInternal(binaryExpression.Left, filterBuilder); - filterBuilder = filterBuilder.Or(); - return BuildFilterFromExpressionInternal(binaryExpression.Right, filterBuilder); + return BuildBooleanExpression(binaryExpression, filterBuilder, or: true); case ExpressionType.Equal: // x => x.Property == value @@ -375,6 +371,29 @@ unaryExpression.Operand is MemberExpression notMemberExpression && throw new NotSupportedException($"The expression type {expression.NodeType} is not supported."); } + /// + /// Emits an && / || node inside its own parentheses. + /// + /// C# expression trees carry precedence in their shape, but the emitted SQL is a flat token + /// stream where AND binds tighter than OR. Without the parentheses, + /// (a || b) && c would flatten to a OR b AND c — read by SQL as + /// a OR (b AND c) — and rows matching only a would come back despite failing + /// c. Grouping each composite node restores the shape the caller wrote. + /// + /// + private FilterBuilder BuildBooleanExpression( + BinaryExpression binaryExpression, + FilterBuilder? filterBuilder, + bool or) + { + filterBuilder = (filterBuilder ?? FilterBuilder.Create()).StartGroup(); + filterBuilder = BuildFilterFromExpressionInternal(binaryExpression.Left, filterBuilder); + filterBuilder = or ? filterBuilder.Or() : filterBuilder.And(); + filterBuilder = BuildFilterFromExpressionInternal(binaryExpression.Right, filterBuilder); + + return filterBuilder.EndGroup(); + } + private FilterBuilder HandleComparisonExpression( BinaryExpression binaryExpression, FilterBuilder filterBuilder, FilterType filterType)