From 173a700e92dda097af2c7963b597505c6660621e Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:38:20 -0500 Subject: [PATCH 01/11] fix: bind the caller's filter as a single conjunct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FilterBuilder appended the caller's terms bare, so the generated clause read "FullTypeName = ? AND Partition = ? AND term1 OR term2". AND binds tighter than OR, so SQL parsed that as "(FullTypeName AND Partition AND term1) OR (term2)": every term after the first Or() was matched against the whole table. A two-term Or() returned rows from other partitions, and rows of other stored types, which the reader deserialized as T with no error. The same clause backs DeleteObjectsAsync, so an ungrouped Or() could delete rows in other partitions and of other types, and CountObjectsAsync over-counted. The caller's filter is now emitted inside its own parentheses. Losing the Partition predicate also cost the partition-prefixed indexes, so this was a large performance regression too; a test asserts via EXPLAIN QUERY PLAN that a two-term OR-chain reaches the index again. TychoQueryable had the same failure one level down: && and || were emitted flat, so Where(x => (x.A || x.B) && x.C) became "A OR B AND C" — read as "A OR (B AND C)" — returning rows that matched only A despite failing C. Each composite boolean node is now emitted in its own group. Every regression test here seeds more than one partition and more than one type: a single-partition, single-type fixture cannot observe the difference, which is presumably why this went unnoticed. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 25 ++ README.md | 11 + TychoDB.UnitTests/FilterCompositionTests.cs | 308 ++++++++++++++++++++ TychoDB/FilterBuilder.cs | 19 +- TychoDB/TychoQueryable.cs | 31 +- 5 files changed, 386 insertions(+), 8 deletions(-) create mode 100644 TychoDB.UnitTests/FilterCompositionTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index fed3db3..1ecc7bc 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 @@ -169,6 +191,9 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Breaking changes +- **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 diff --git a/README.md b/README.md index 5431d33..f42030d 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,17 @@ var complexFilter = FilterBuilder .And() .Filter(FilterType.Contains, x => x.Name, "Doe"); +// 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); 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/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index 9a9ea4d..0a9fd7b 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -112,11 +112,24 @@ public FilterBuilder EndGroup() internal void Build(StringBuilder commandBuilder, IJsonSerializer jsonSerializer, FilterParameters parameters) { - 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); @@ -157,6 +170,8 @@ internal void Build(StringBuilder commandBuilder, IJsonSerializer jsonSerializer BuildSimpleFilter(commandBuilder, filter, jsonSerializer, parameters); } } + + commandBuilder.AppendLine(CloseParen); } /// 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) From d37fb7393e67d65fa53f00384c811de28fd53c82 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:39:08 -0500 Subject: [PATCH 02/11] feat: add FilterType.In and FilterType.NotIn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set membership expressed as one atomic term rather than a chain of Or()s, so the whole class of mis-grouping the previous commit fixed is unreachable for this query shape. It renders through the same numeric CAST the scalar comparisons use, which is what lets an expression index over the property serve the query. FilterBuilder.Create().Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 }); Decisions worth reviewing: - An empty set renders "0 = 1" for In and "1 = 1" for NotIn. IN () is a syntax error, and silently dropping the term would widen the result set, which is the dangerous direction. - A null in the set becomes an IS NULL disjunct, which SQL's own IN would never match. NotIn keeps SQL's NULL semantics — a row whose member is null is not returned — because that is what NotEquals already does, and diverging from its scalar sibling would be its own trap. A test asserts the two agree. - Lists longer than 900 values split across several IN terms rather than exceeding SQLITE_MAX_VARIABLE_NUMBER, which is 999 on older SQLite builds. Which build ships is the host application's choice, not ours. - The raw-path overload takes IEnumerable, not a generic parameter: a generic overload there captures an ordinary string comparison value, because string is IEnumerable. A value-type collection therefore needs Cast(), and both the parameter docs and the rejection message say so. Passing a collection to a scalar FilterType now throws instead of rendering "System.Int32[]" and matching nothing. A literal null still binds to the new overload but keeps its old meaning as the null comparison; two existing tests caught that when I first got it wrong, and there is now a test naming it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 29 ++ README.md | 5 + TychoDB.UnitTests/FilterInTests.cs | 507 +++++++++++++++++++++++++++++ TychoDB/Filter.cs | 12 + TychoDB/FilterBuilder.cs | 353 +++++++++++++++++++- 5 files changed, 905 insertions(+), 1 deletion(-) create mode 100644 TychoDB.UnitTests/FilterInTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ecc7bc..c28c15b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -177,6 +177,29 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Added +- **`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. + - The raw-path overload takes `IEnumerable` rather than a generic parameter on + purpose: a generic overload there captures an ordinary `string` comparison value, since + `string` is an `IEnumerable`. A value-type collection needs `Cast()`; the + expression overload infers the element type from the property and needs no cast. - **`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 @@ -191,6 +214,12 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Breaking changes +- **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. diff --git a/README.md b/README.md index f42030d..115b517 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,11 @@ 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() diff --git a/TychoDB.UnitTests/FilterInTests.cs b/TychoDB.UnitTests/FilterInTests.cs new file mode 100644 index 0000000..38f128d --- /dev/null +++ b/TychoDB.UnitTests/FilterInTests.cs @@ -0,0 +1,507 @@ +#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 async Task RawPathOverload_TakesAValueTypeCollectionViaCast() + { + // IEnumerable is deliberate: a generic overload here would capture an ordinary + // string comparison value, because string is IEnumerable. The cost is that a + // value-type collection needs Cast(), and the error when it is omitted has to + // say so. + using var db = Connect(out _); + await SeedAsync(db); + + var results = + await db.ReadObjectsAsync( + PartitionA, + FilterBuilder.Create().Filter( + FilterType.In, "$.DepartmentId", true, false, false, new[] { 33, 47 }.Cast())); + + results.Select(x => x.Id).OrderBy(x => x).ShouldBe(new[] { 1, 2 }); + } + + [TestMethod] + public void RawPathOverload_WithAnUncastValueTypeCollection_SaysWhatToDo() + { + // int[] binds to the scalar object overload, which rejects a set filter type loudly + // rather than rendering "System.Int32[]" and matching nothing. + var build = () => + FilterBuilder.Create().Filter(FilterType.In, "$.DepartmentId", true, false, false, new[] { 33, 47 }); + + Should.Throw(build).Message.ShouldContain("Cast()"); + } + + [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/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 0a9fd7b..20c5a54 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -34,6 +34,26 @@ 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 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 +68,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 +84,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 +99,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 +112,172 @@ 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. Typed as of + /// rather than a generic parameter on purpose: a generic overload here + /// would capture an ordinary comparison value, since + /// is an of , and route + /// it to set membership. A value-type collection such as int[] therefore needs + /// Cast<object>(); the expression overload infers the element type from the + /// property and needs no cast. + /// + /// 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. " + + "The raw-path overload takes IEnumerable, which a value-type collection such as int[] " + + "does not implicitly convert to — call Cast() on it.", + 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)); @@ -186,7 +378,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) { @@ -236,6 +428,36 @@ private static Filter Rebuild(in Filter filter, string? propertyPath, string? pr value); } + /// + /// 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; + } + /// /// Converts a comparison value into the form the serializer writes into the document, so /// the comparison is made against what is actually stored. Only values that would @@ -601,6 +823,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); } } From c9c5bd8cb7f77ac84a0bfa3c614a46e79dd98760 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:22:00 -0500 Subject: [PATCH 03/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB/FilterBuilder.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/TychoDB/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index 20c5a54..4676752 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -46,13 +46,10 @@ public class FilterBuilder 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. + // Values bind as parameters unless they are genuine numerics or booleans (those become literals). + // SQLite's SQLITE_MAX_VARIABLE_NUMBER limit is statement-wide (total bound variables in the SQL). + // Chunking a large set across multiple IN (...) terms does not reduce the parameter count for + // parameterized values (e.g., strings / enums / DateTime), but it keeps each IN list reasonably sized. private const int MaxValuesPerInClause = 900; private readonly List _filters = new(); From ce98d5ed08af42f23080d8fea637ceaf7c788de2 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:22:20 -0500 Subject: [PATCH 04/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB/FilterBuilder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/TychoDB/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index 4676752..37b451e 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -118,9 +118,9 @@ public FilterBuilder Filter(FilterType filterType, string propertyPath, bo /// /// /// 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. - /// + /// and everything for . If the set contains , + /// adds an IS NULL disjunct; excludes + /// missing/null members (like ). /// /// The property's type. /// Must be or . From da3d65e50d651a997965e257015fb9a44fb13a60 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:22:41 -0500 Subject: [PATCH 05/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB/FilterBuilder.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/TychoDB/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index 37b451e..7e547ef 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -237,8 +237,7 @@ private static void EnsureScalarFilterType(FilterType filterType) { throw new ArgumentException( $"{filterType} tests set membership; use the overload that takes an IEnumerable of values. " + - "The raw-path overload takes IEnumerable, which a value-type collection such as int[] " + - "does not implicitly convert to — call Cast() on it.", + "For raw JSON paths, the collection overload takes IEnumerable; value-type collections (e.g., int[]) need Cast().", nameof(filterType)); } } From 76e6f5401a5bb6dbdbc346b033347643f3c77742 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:23:00 -0500 Subject: [PATCH 06/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB.UnitTests/FilterInTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/TychoDB.UnitTests/FilterInTests.cs b/TychoDB.UnitTests/FilterInTests.cs index 38f128d..ed3b9fe 100644 --- a/TychoDB.UnitTests/FilterInTests.cs +++ b/TychoDB.UnitTests/FilterInTests.cs @@ -232,10 +232,10 @@ await db.WriteObjectsAsync( } [TestMethod] - public async Task In_WithMoreValuesThanTheParameterCeiling_StillMatches() + public async Task In_WithManyValues_StillMatches() { - // Long lists are split across several IN terms rather than exceeding - // SQLITE_MAX_VARIABLE_NUMBER, which is 999 on older builds. + // Long lists are split across several IN terms so each IN list stays reasonably sized. + // (Note: this does not reduce the total number of bound parameters for parameterized values.) using var db = Connect(out _); await SeedAsync(db); From a1d2c0a041ec6f2dd6a40d81c5f6485cabdd8ab4 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:23:16 -0500 Subject: [PATCH 07/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c28c15b..3ab03c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -193,9 +193,9 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 - 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. + - Longer lists are split across several `IN` terms to keep each `IN (...)` list reasonably sized. + (Note: SQLite's `SQLITE_MAX_VARIABLE_NUMBER` limit is statement-wide; very large parameterized + value sets (e.g., strings) can still exceed it on older builds.) - The raw-path overload takes `IEnumerable` rather than a generic parameter on purpose: a generic overload there captures an ordinary `string` comparison value, since `string` is an `IEnumerable`. A value-type collection needs `Cast()`; the From aa2cd40725a5320165b7b1e3b2ff29c00716d67b Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:40:22 -0500 Subject: [PATCH 08/11] feat: add ReadObjectsByKeysAsync, and count in the engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch key reads --------------- Reading N keys meant N round trips through the connection gate. The new overload does it in one, binding the key set as a single JSON array expanded by JSON_EACH rather than one parameter per key — so there is no SQLITE_MAX_VARIABLE_NUMBER ceiling, no chunking for callers to think about, and one prepared statement whatever the batch size. Keys lead the PRIMARY KEY, so each expanded key is a primary-key probe. The shape was chosen by measurement, not assumption. Query cost alone on a 250,000-row store: batch json_each chunked IN single IN temp table 200 0.2 ms 0.3 ms 0.3 ms 40.3 ms 999 1.0 ms 3.3 ms 4.0 ms 47.3 ms 4,949 5.6 ms 19.6 ms 65.8 ms 52.9 ms 23,784 27.5 ms 91.8 ms 1,297.7 ms 67.3 ms A single IN collapses at scale because its statement text and plan grow with the batch; a temp-table join carries fixed setup that never amortises at these sizes. End to end against a loop of ReadObjectAsync, both including deserialization: 1.9 -> 0.9 ms at 200 keys, 10.6 -> 4.5 at 999, 36.8 -> 16.9 at 4,949, 183.2 -> 67.3 at 23,784. Keys not present are simply absent from the result, so it may be shorter than the key set, and its order is the database's rather than the key set's. Tests cover what the JSON encoding could break: keys carrying quotes, backslashes, control characters, non-BMP emoji and a SQL injection string, plus a 5,000-key set. Counting -------- CountObjectsAsync issued "SELECT 1 FROM JsonValue WHERE ..." and incremented a counter once per matching row — 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. The same query backs the pre-count a progress-reporting read performs, so those pay half of what they did. The benchmark harness is committed as ZzBatchKeyBench.cs, [Ignore]d so it never runs in CI; remove the attribute to reproduce any number above. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 35 +++ README.md | 24 ++ TychoDB.UnitTests/BatchKeyReadTests.cs | 272 +++++++++++++++++ TychoDB.UnitTests/ZzBatchKeyBench.cs | 405 +++++++++++++++++++++++++ TychoDB/Queries.cs | 37 ++- TychoDB/Tycho.cs | 129 +++++++- 6 files changed, 889 insertions(+), 13 deletions(-) create mode 100644 TychoDB.UnitTests/BatchKeyReadTests.cs create mode 100644 TychoDB.UnitTests/ZzBatchKeyBench.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ab03c7..e11c9fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,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 @@ -177,6 +186,26 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4 ### Added +- **`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`: @@ -269,6 +298,12 @@ 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. + - 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 115b517..1ea8fc8 100644 --- a/README.md +++ b/README.md @@ -190,8 +190,19 @@ 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`) 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 ```csharp @@ -339,6 +350,19 @@ var result = await db.DeleteBlobsAsync("documents"); Console.WriteLine($"Deleted {result.Count} blobs"); ``` +## Performance notes + +- **Reach rows by key through the key APIs.** A filter on the key property scans; see the note + under [Basic Querying](#basic-querying). `ReadObjectsByKeysAsync` fetches a whole batch in one + round trip and has no limit on batch size. +- **`SystemTextJsonSerializer` deserializes faster.** It implements `IUtf8JsonDeserializer`, so + rows are handed to it as UTF-8 spans and skip an intermediate stream. Reading a whole + 250,000-row partition measured **254.7 ms** with `SystemTextJsonSerializer` against + **358.9 ms** with `NewtonsoftJsonSerializer` — about 1.4x. Deserialization dominates any large + read, so this is usually the largest single lever on read throughput. +- **Index anything you filter or sort on.** An unindexed `JSON_EXTRACT` predicate scans the + partition; see below. + ## Indexing Create indexes to improve query performance: diff --git a/TychoDB.UnitTests/BatchKeyReadTests.cs b/TychoDB.UnitTests/BatchKeyReadTests.cs new file mode 100644 index 0000000..b71b022 --- /dev/null +++ b/TychoDB.UnitTests/BatchKeyReadTests.cs @@ -0,0 +1,272 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; + +namespace TychoDB.UnitTests; + +/// +/// Reading a batch of keys in one round trip. The key set is bound as a JSON array rather than +/// one parameter per key, so the tests that matter most are the ones that would break that +/// encoding: keys carrying JSON metacharacters, and key sets larger than SQLite's parameter +/// ceiling. +/// +[TestClass] +public class BatchKeyReadTests +{ + private const string PartitionA = "partitionA"; + private const string PartitionB = "partitionB"; + + [TestMethod] + public async Task ReadsTheObjectsForTheGivenKeys() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "k3" }, PartitionA); + + results.Select(x => x.Key).OrderBy(x => x).ShouldBe(new[] { "k1", "k3" }); + } + + [TestMethod] + public async Task KeysThatAreNotPresent_AreSimplyAbsent() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "nope", "k2" }, PartitionA); + + results.Select(x => x.Key).OrderBy(x => x).ShouldBe(new[] { "k1", "k2" }); + } + + [TestMethod] + public async Task EmptyKeySet_ReturnsNothing() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(Array.Empty(), PartitionA); + + results.ShouldBeEmpty(); + } + + [TestMethod] + public async Task StaysWithinItsPartition() + { + using var db = Connect(); + await SeedAsync(db); + + // Both keys exist in both partitions, holding different values; only partition A's + // versions may come back. + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "shared" }, PartitionA); + + results.Select(x => x.Description).OrderBy(x => x, StringComparer.Ordinal) + .ShouldBe(new[] { "a-shared", "a1" }); + } + + [TestMethod] + public async Task StaysWithinItsType() + { + // Same key, same partition, different stored type. + using var db = Connect(); + await SeedAsync(db); + await db.WriteObjectsAsync(new[] { new Other { Key = "k1", Note = "wrong type" } }, x => x.Key, PartitionA); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1" }, PartitionA); + + results.Select(x => x.Description).ShouldBe(new[] { "a1" }); + } + + [TestMethod] + public async Task DuplicateKeys_YieldOneObjectEach() + { + using var db = Connect(); + await SeedAsync(db); + + var results = await db.ReadObjectsByKeysAsync(new object[] { "k1", "k1", "k1" }, PartitionA); + + results.Select(x => x.Key).ShouldBe(new[] { "k1" }); + } + + [TestMethod] + public async Task KeysCarryingJsonMetacharacters_RoundTrip() + { + // The key set is rendered as JSON, so a quote, a backslash, a control character or a + // non-BMP character in a key would corrupt the array if it were not properly encoded. + var hostile = new[] + { + "he said \"hi\"", + @"back\slash", + "tab\tand\nnewline", + "emoji \U0001F389", + "unicode é中", + "'; DROP TABLE JsonValue; --", + }; + + using var db = Connect(); + await db.WriteObjectsAsync( + hostile.Select(k => new Doc { Key = k, Description = "held" }), x => x.Key, PartitionA); + + var results = await db.ReadObjectsByKeysAsync(hostile.Cast(), PartitionA); + + results.Select(x => x.Key).OrderBy(x => x, StringComparer.Ordinal) + .ShouldBe(hostile.OrderBy(x => x, StringComparer.Ordinal)); + } + + [TestMethod] + public async Task KeySetLargerThanTheParameterCeiling_Works() + { + // One bound parameter regardless of key count, so SQLITE_MAX_VARIABLE_NUMBER — 999 on + // older builds — does not apply and no chunking is needed. + const int count = 5_000; + + using var db = Connect(); + await db.WriteObjectsAsync( + Enumerable.Range(0, count).Select(i => new Doc + { + Key = "b" + i.ToString(CultureInfo.InvariantCulture), + Description = "bulk", + }), + x => x.Key, + PartitionA); + + var keys = Enumerable.Range(0, count).Select(i => (object)("b" + i.ToString(CultureInfo.InvariantCulture))); + + var results = await db.ReadObjectsByKeysAsync(keys, PartitionA); + + results.Count().ShouldBe(count); + } + + [TestMethod] + public async Task NonStringKeys_MatchTheSingleKeyOverload() + { + using var db = Connect(); + await db.WriteObjectsAsync( + new[] { new Numbered { Id = 7, Note = "seven" }, new Numbered { Id = 8, Note = "eight" } }, + x => x.Id, + PartitionA); + + var batch = await db.ReadObjectsByKeysAsync(new object[] { 7, 8 }, PartitionA); + var single = await db.ReadObjectAsync(7, PartitionA); + + batch.Select(x => x.Note).OrderBy(x => x).ShouldBe(new[] { "eight", "seven" }); + single.Note.ShouldBe("seven"); + } + + [TestMethod] + public async Task AppliesSorting() + { + using var db = Connect(); + await SeedAsync(db); + + var results = + await db.ReadObjectsByKeysAsync( + new object[] { "k1", "k2", "k3" }, + PartitionA, + SortBuilder.Create().OrderBy(SortDirection.Descending, x => x.Description)); + + results.Select(x => x.Description).ShouldBe(new[] { "a3", "a2", "a1" }); + } + + [TestMethod] + public async Task ReportsProgress() + { + using var db = Connect(); + await SeedAsync(db); + + // A synchronous reporter, not Progress, which dispatches its callbacks + // asynchronously and would make this a race. + var progress = new RecordingProgress(); + + await db.ReadObjectsByKeysAsync(new object[] { "k1", "k2", "k3" }, PartitionA, progress: progress); + + progress.Reports.ShouldNotBeEmpty(); + progress.Reports[^1].ShouldBe(1.0); + } + + [TestMethod] + public async Task NullKeyInTheSet_IsRejected() + { + using var db = Connect(); + await SeedAsync(db); + + await Should.ThrowAsync( + async () => await db.ReadObjectsByKeysAsync(new object?[] { "k1", null }!, PartitionA)); + } + + [TestMethod] + public async Task NullKeySet_IsRejected() + { + using var db = Connect(); + await SeedAsync(db); + + await Should.ThrowAsync( + async () => await db.ReadObjectsByKeysAsync(null!, PartitionA)); + } + + private static Tycho Connect() + { + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + + var db = new Tycho(dir, new NewtonsoftJsonSerializer(), dbName: name, rebuildCache: true, requireTypeRegistration: false); + return db.Connect(); + } + + private static async Task SeedAsync(Tycho db) + { + await db.WriteObjectsAsync( + new[] + { + new Doc { Key = "k1", Description = "a1" }, + new Doc { Key = "k2", Description = "a2" }, + new Doc { Key = "k3", Description = "a3" }, + new Doc { Key = "shared", Description = "a-shared" }, + }, + x => x.Key, + PartitionA); + + await db.WriteObjectsAsync( + new[] + { + new Doc { Key = "k1", Description = "b1" }, + new Doc { Key = "shared", Description = "b-shared" }, + }, + x => x.Key, + PartitionB); + } + + private sealed class RecordingProgress : IProgress + { + public List Reports { get; } = new(); + + public void Report(double value) => Reports.Add(value); + } + + public class Doc + { + public string Key { get; set; } = string.Empty; + + public string Description { get; set; } = string.Empty; + } + + public class Other + { + public string Key { get; set; } = string.Empty; + + public string Note { get; set; } = string.Empty; + } + + public class Numbered + { + public int Id { get; set; } + + public string Note { get; set; } = string.Empty; + } +} diff --git a/TychoDB.UnitTests/ZzBatchKeyBench.cs b/TychoDB.UnitTests/ZzBatchKeyBench.cs new file mode 100644 index 0000000..132f3f3 --- /dev/null +++ b/TychoDB.UnitTests/ZzBatchKeyBench.cs @@ -0,0 +1,405 @@ +#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); + } + } + + 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/Queries.cs b/TychoDB/Queries.cs index e752f9d..123072e 100644 --- a/TychoDB/Queries.cs +++ b/TychoDB/Queries.cs @@ -235,14 +235,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/Tycho.cs b/TychoDB/Tycho.cs index c866ec9..fe432d6 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", @@ -558,12 +560,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 +846,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 +945,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, CancellationToken cancellationToken)>( _connectionGate, - (partition, filter, sort, top, withTransaction, progress, _commandBuilder, _commandTimeout, _jsonSerializer, cancellationToken), + (partition, filter, sort, top, withTransaction, progress, _commandBuilder, _commandTimeout, _jsonSerializer, keysJson, cancellationToken), static async (conn, state) => { SqliteTransaction? transaction = null; @@ -870,10 +958,14 @@ 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(); + if (state.filter is not null) { state.filter.Build(commandBuilder, state.jsonSerializer, filterParameters); @@ -904,6 +996,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,7 +1011,10 @@ 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) @@ -930,12 +1030,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) From 5aa9800c7d9a2e1a9177d9fcccc884106a765dc0 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:25:18 -0500 Subject: [PATCH 09/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB/Tycho.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TychoDB/Tycho.cs b/TychoDB/Tycho.cs index fe432d6..130ad22 100644 --- a/TychoDB/Tycho.cs +++ b/TychoDB/Tycho.cs @@ -561,7 +561,7 @@ public ValueTask CountObjectsAsync(string? partition = null, FilterBuild using var reader = selectCommand.ExecuteReader(); // One row holding the count, rather than one row per match. - int count = reader.Read() ? reader.GetInt32(0) : 0; + int count = reader.Read() ? checked((int)reader.GetInt64(0)) : 0; transaction?.Commit(); From 4ca1b6c03a66ebe3aab2a9a36eaf942362ddbd4d Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:47:38 -0500 Subject: [PATCH 10/11] fix: restore a closing dropped by an autofix ce98d5e rewrote the set-membership doc block and removed its without removing the matching , leaving the summary unbalanced. That produced two CS1570 "badly formed XML" warnings (four counting the duplicated build output) on this branch and everything downstream of it. The prose the autofix introduced is kept; only the tag is restored. Co-Authored-By: Claude Opus 5 --- TychoDB/FilterBuilder.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/TychoDB/FilterBuilder.cs b/TychoDB/FilterBuilder.cs index 7e547ef..c81b23a 100644 --- a/TychoDB/FilterBuilder.cs +++ b/TychoDB/FilterBuilder.cs @@ -121,6 +121,7 @@ public FilterBuilder Filter(FilterType filterType, string propertyPath, bo /// and everything for . If the set contains , /// adds an IS NULL disjunct; excludes /// missing/null members (like ). + /// /// /// The property's type. /// Must be or . From 25af528185ba28f199eddfa836d2a8d252502686 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:56:41 -0500 Subject: [PATCH 11/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB/Tycho.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TychoDB/Tycho.cs b/TychoDB/Tycho.cs index 130ad22..6952fb9 100644 --- a/TychoDB/Tycho.cs +++ b/TychoDB/Tycho.cs @@ -910,7 +910,7 @@ public ValueTask> ReadObjectsByKeysAsync( var any = false; foreach (var key in keys) { - ArgumentNullException.ThrowIfNull(key, nameof(keys)); + ArgumentNullException.ThrowIfNull(key, nameof(key)); writer.WriteStringValue(key.ToString()); any = true;