diff --git a/CHANGELOG.md b/CHANGELOG.md index e11c9fb..d412978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -186,6 +186,40 @@ 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 @@ -243,15 +277,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. -- **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. - **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 @@ -304,6 +344,7 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 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. + - 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 1ea8fc8..8452fbe 100644 --- a/README.md +++ b/README.md @@ -196,12 +196,13 @@ var count = await db.CountObjectsAsync(); 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`) goes through -> `JSON_EXTRACT` and scans — it does not use the primary key, because Tycho stores the key in -> its own `Key` column and cannot assume the property still matches it (a write may supply its -> own key selector). Reach those rows through `ReadObjectAsync` / `ReadObjectsByKeysAsync`, or -> index the property like any other. On a 250,000-row store, one equality lookup measured -> 71.6 ms as an unindexed filter and 0.0 ms all three other ways. +> **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 diff --git a/TychoDB.UnitTests/KeyRegistrationTests.cs b/TychoDB.UnitTests/KeyRegistrationTests.cs new file mode 100644 index 0000000..dfcd774 --- /dev/null +++ b/TychoDB.UnitTests/KeyRegistrationTests.cs @@ -0,0 +1,463 @@ +#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 void ConventionRegistration_IgnoresAPropertyWithoutAPublicGetter() + { + // A public setter is enough for the property to show up in a Public lookup, and the + // expression selector would even compile against a private getter — but neither + // serializer writes such a property, so its JSON path would match nothing. + using var db = Connect(t => t.AddTypeRegistration()); + + Should.Throw(() => db.GetIdFor(new PrivateGetterId())).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_WithMoreValuesThanOneChunk_StaysBoundToItsOwnTerm() + { + // A set larger than one chunk emits several "Key IN (...)" terms joined by OR. AND binds + // tighter than OR, so if that chain is not wrapped, a following AND term captures only + // the last chunk and every id in the earlier chunks comes back regardless of it — the + // same precedence bug the enclosing parentheses exist to prevent, one level in. + using var db = Connect(t => t.AddTypeRegistration(x => x.Id), strict: true); + + const int count = 1_500; + await db.WriteObjectsAsync( + Enumerable.Range(0, count).Select(i => new Doc + { + Id = "id-" + i.ToString("D5", CultureInfo.InvariantCulture), + Value = i % 2 == 0 ? "keep" : "drop", + }), + x => x.Id, + Partition); + + var everyId = + Enumerable.Range(0, count).Select(i => "id-" + i.ToString("D5", CultureInfo.InvariantCulture)); + + var results = + await db.ReadObjectsAsync( + Partition, + FilterBuilder.Create() + .Filter(FilterType.In, x => x.Id, everyId).And() + .Filter(FilterType.Equals, x => x.Value, "keep")); + + results.Count().ShouldBe(count / 2); + results.ShouldAllBe(x => x.Value == "keep"); + } + + [TestMethod] + public async Task Reregistration_DoesNotLeaveAStaleRewritePathCached() + { + // The rewrite caches the resolved id path per type. Re-registering with a different id + // property must not leave filters pointing at the old one. + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + + var tycho = new Tycho(dir, new NewtonsoftJsonSerializer(), dbName: name, rebuildCache: true, requireTypeRegistration: true) + .AddTypeRegistration(x => x.Id); + + using var db = tycho.Connect(); + await SeedAsync(db); + + // Warm the cache against $.Id. + await db.ReadObjectsAsync( + Partition, FilterBuilder.Create().Filter(FilterType.Equals, x => x.Id, "id-2")); + + // Re-register against a different property; rows are still keyed by Id. + db.AddTypeRegistration(x => x.Value); + + // A filter on Value must not be answered from the Key column, which holds Ids. + 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 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", 30)); + + 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 PrivateGetterId + { + public string Id { private get; set; } = "unreachable"; + + public string Value { get; set; } = string.Empty; + } + + public class Indexed + { + public string this[int i] => i.ToString(CultureInfo.InvariantCulture); + + public string Value { get; set; } = string.Empty; + } +} diff --git a/TychoDB.UnitTests/ZzBatchKeyBench.cs b/TychoDB.UnitTests/ZzBatchKeyBench.cs index 132f3f3..1b03ebc 100644 --- a/TychoDB.UnitTests/ZzBatchKeyBench.cs +++ b/TychoDB.UnitTests/ZzBatchKeyBench.cs @@ -251,6 +251,68 @@ await Timed("filter on key property, indexed", async () => } } + [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(); diff --git a/TychoDB/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index c81b23a..0387360 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -38,6 +38,7 @@ public class FilterBuilder 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 @@ -299,7 +300,7 @@ 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) { @@ -356,6 +357,11 @@ 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); } } @@ -717,6 +723,96 @@ 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; + } + + // A chunked chain is several IN terms joined by OR, and AND binds tighter than + // OR: left unwrapped, a following AND term would capture only the last chunk — + // "Key IN (a) OR Key IN (b) AND other" reads as "Key IN (a) OR (Key IN (b) AND + // other)". That is the same precedence bug the enclosing parentheses in Build + // exist to prevent, one level further in, so this chain is bound together too. + // BuildSetFilter does the same for the JSON-path form. + var chunks = ((values.Length - 1) / MaxValuesPerInClause) + 1; + var wrap = chunks > 1; + + if (wrap) + { + commandBuilder.Append(OpenParen); + } + + for (var chunk = 0; chunk < chunks; 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); + } + + if (wrap) + { + 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) diff --git a/TychoDB/KeyColumnRewrite.cs b/TychoDB/KeyColumnRewrite.cs new file mode 100644 index 0000000..b71e61b --- /dev/null +++ b/TychoDB/KeyColumnRewrite.cs @@ -0,0 +1,79 @@ +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 — it runs under the connection's configured command timeout, and its verdict is +/// cached for the lifetime of the connection. +/// +/// +internal sealed class KeyColumnRewrite +{ + private readonly ConcurrentDictionary _usableByTypeName = new(StringComparer.Ordinal); + + private readonly int _commandTimeout; + + public KeyColumnRewrite(string resolvedIdPath, int commandTimeout) + { + ResolvedIdPath = resolvedIdPath; + _commandTimeout = commandTimeout; + } + + /// + /// 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 + + // The probe is a scan, which makes it the command most likely to need the caller's + // raised timeout rather than the provider default. + command.CommandTimeout = _commandTimeout; + 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 123072e..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 diff --git a/TychoDB/RegisteredTypeInformation.cs b/TychoDB/RegisteredTypeInformation.cs index dcc7904..b8e0c97 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,96 @@ 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. + // + // CanRead is not enough — it is true for a private getter, and a property with a + // public setter and a private getter is still returned by a Public lookup. Such + // a property would compile into a working selector, but neither serializer emits + // it, so the id would be absent from the stored document: the JSON path would + // match nothing and every row would look divergent to the key-column rewrite's + // probe. Requiring a public getter keeps the convention aligned with what is + // actually written to the document. + if (property.GetIndexParameters().Length > 0 + || property.GetMethod is not { IsPublic: true }) + { + 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 6952fb9..3086ac1 100644 --- a/TychoDB/Tycho.cs +++ b/TychoDB/Tycho.cs @@ -47,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(); @@ -170,6 +173,12 @@ public Tycho AddTypeRegistration( _registeredTypeInformation[rti.ObjectType] = rti; + // The rewrite caches the id path resolved from the previous registration. Re-registering + // a type can change that path — or remove the id property altogether — so the cached + // entry has to go, or filters would be rewritten against a path the stored keys no + // longer come from. + _keyColumnRewrites.TryRemove(rti.ObjectType, out _); + return this; } @@ -186,6 +195,12 @@ public Tycho AddTypeRegistration() _registeredTypeInformation[rti.ObjectType] = rti; + // The rewrite caches the id path resolved from the previous registration. Re-registering + // a type can change that path — or remove the id property altogether — so the cached + // entry has to go, or filters would be rewritten against a path the stored keys no + // longer come from. + _keyColumnRewrites.TryRemove(rti.ObjectType, out _); + return this; } @@ -205,6 +220,12 @@ public Tycho AddTypeRegistrationWithCustomKeySelector( _registeredTypeInformation[rti.ObjectType] = rti; + // The rewrite caches the id path resolved from the previous registration. Re-registering + // a type can change that path — or remove the id property altogether — so the cached + // entry has to go, or filters would be rewritten against a path the stored keys no + // longer come from. + _keyColumnRewrites.TryRemove(rti.ObjectType, out _); + return this; } @@ -332,6 +353,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) { @@ -363,6 +398,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) { @@ -370,6 +419,8 @@ public ValueTask WriteObjectsAsync(IEnumerable objs, Func ArgumentNullException.ThrowIfNull(keySelector); ArgumentNullException.ThrowIfNull(_connection); + keySelector = GuardAgainstKeyDivergence(keySelector); + return _connection .WithConnectionBlockAsync( _connectionGate, @@ -527,7 +578,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; @@ -550,7 +601,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. @@ -945,9 +1000,9 @@ private ValueTask> ReadObjectsCoreAsync( ArgumentNullException.ThrowIfNull(_connection); return _connection - .WithConnectionBlockAsync, (string? partition, FilterBuilder? filter, SortBuilder? sort, int? top, bool withTransaction, IProgress? progress, StringBuilder commandBuilder, int commandTimeout, IJsonSerializer jsonSerializer, string? keysJson, 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, keysJson, cancellationToken), + (partition, filter, sort, top, withTransaction, progress, _commandBuilder, _commandTimeout, _jsonSerializer, keysJson, GetKeyColumnRewriteFor(), cancellationToken), static async (conn, state) => { SqliteTransaction? transaction = null; @@ -966,9 +1021,13 @@ state.keysJson is null // 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) @@ -1019,7 +1078,7 @@ state.keysJson is null 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(); @@ -1228,9 +1287,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; @@ -1257,7 +1316,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. @@ -1412,7 +1475,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; @@ -1435,7 +1498,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. @@ -2503,6 +2570,89 @@ 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)), + state.CommandTimeout), + (Segments: rti.IdPropertyPathSegments, Serializer: _jsonSerializer, CommandTimeout: _commandTimeout)); + } + + /// + /// 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. ///