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.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 +/// 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/TychoDbTests.cs b/TychoDB.UnitTests/TychoDbTests.cs index a44ea61..c07d5a3 100644 --- a/TychoDB.UnitTests/TychoDbTests.cs +++ b/TychoDB.UnitTests/TychoDbTests.cs @@ -1164,7 +1164,11 @@ public async Task TychoDb_QueryUsingContains_ShouldBeSuccessful(IJsonSerializer }) .ToList(); - await db.WriteObjectsAsync(testObjs, x => 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(); 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)