diff --git a/src/CatDb/Data/Comparer.cs b/src/CatDb/Data/Comparer.cs index 7b456d9..f39b5b5 100644 --- a/src/CatDb/Data/Comparer.cs +++ b/src/CatDb/Data/Comparer.cs @@ -53,7 +53,6 @@ public static Expression CreateComparerBody(List? expressions, List< var exitPoint = Expression.Label(typeof(int)); var list = new List(); var variables = new List(); - var haveCmp = false; if (expressions != null) foreach (var expression in expressions) @@ -70,14 +69,15 @@ public static Expression CreateComparerBody(List? expressions, List< { var i = 0; var cmp = Expression.Variable(typeof(int)); + // The `cmp` temp is assigned by every non-last member compare (numeric, Guid, DateTime, + // char, byte[], decimal, string, …), so it must always be declared in the block scope. + // The old `haveCmp` gate only registered it for char/byte[]/decimal/string members, so a + // composite whose non-last members are e.g. Guid/int/DateTime (a non-unique index key over + // a Guid-keyed table builds Slots) referenced an undeclared variable and the + // expression tree failed to compile. + variables.Add(cmp); foreach (var field in DataTypeUtils.GetPublicMembers(x.Type, membersOrder)) { - if (!haveCmp && (field.GetPropertyOrFieldType() == typeof(char) || field.GetPropertyOrFieldType() == typeof(byte[]) || field.GetPropertyOrFieldType() == typeof(Decimal) || field.GetPropertyOrFieldType() == typeof(String))) - { - haveCmp = true; - variables.Add(cmp); - } - foreach (var command in GetCompareCommands(Expression.PropertyOrField(x, field.Name), Expression.PropertyOrField(y, field.Name), cmp, exitPoint, field.GetPropertyOrFieldType(), compareOptions[i++], i == DataTypeUtils.GetPublicMembers(x.Type, membersOrder).Count())) list.Add(command); } diff --git a/src/CatDb/Data/DataTypeUtils.cs b/src/CatDb/Data/DataTypeUtils.cs index 8cfc4da..1bafdb8 100644 --- a/src/CatDb/Data/DataTypeUtils.cs +++ b/src/CatDb/Data/DataTypeUtils.cs @@ -41,6 +41,31 @@ public static bool IsAllPrimitive(Type type, Func? member return true; } + /// + /// Like but also treats as a comparable scalar. + /// Used to decide whether a type (e.g. a composite Slots index key) can get a compiled + /// DataComparer/DataEqualityComparer. A non-unique index over a Guid-keyed table + /// builds a composite key Slots(field, Guid); that Guid slot is not "primitive", so + /// returns false and the key would otherwise get no comparer. + /// + public static bool IsAllComparable(Type type, Func? membersOrder = null) + { + if (DataType.IsPrimitiveType(type) || type == typeof(Guid)) + return true; + + if (type.IsArray || type.IsList() || type.IsDictionary() || type.IsKeyValuePair() || type.IsNullable()) + return false; + + foreach (var member in GetPublicMembers(type, membersOrder)) + { + var memberType = member.GetPropertyOrFieldType(); + if (!DataType.IsPrimitiveType(memberType) && memberType != typeof(Guid)) + return false; + } + + return true; + } + private static bool InternalIsAnonymousType(Type type, Func? membersOrder = null) { if (DataType.IsPrimitiveType(type)) diff --git a/src/CatDb/Database/Indexing/SlotAccessor.cs b/src/CatDb/Database/Indexing/SlotAccessor.cs index 98611eb..4ee3b11 100644 --- a/src/CatDb/Database/Indexing/SlotAccessor.cs +++ b/src/CatDb/Database/Indexing/SlotAccessor.cs @@ -276,7 +276,16 @@ internal static Type NormalizeStorageType(Type t) if (t.IsEnum) return t.GetEnumUnderlyingType(); var underlying = Nullable.GetUnderlyingType(t); - return underlying is not null ? NormalizeStorageType(underlying) : t; + if (underlying is not null) + return NormalizeStorageType(underlying); + // Remote/portable tables transport a Nullable as a single-slot Slots (a CLR type the + // Nullable check above can't see). Flatten it to the inner storage type so the index key stays + // a flat scalar/composite — otherwise the key type is a NESTED Slots(Slots, pk) whose + // comparer can't be built (null KeyComparer → NRE on insert → silently dropped rows). See + // RemoteInsertReproTests. + if (TryGetSingleSlotField(t, out var slot0)) + return NormalizeStorageType(slot0.FieldType); + return t; } /// Wraps a value expression so it yields the normalized storage value (see ). @@ -294,9 +303,62 @@ private static Expression NormalizeStorageValue(Expression access) return underlying.IsEnum ? Expression.Convert(value, underlying.GetEnumUnderlyingType()) : value; } + // Portable Nullable representation (single-slot Slots): read the inner slot, then + // normalize it. Matches NormalizeStorageType's flattening above. The Slots wrapper is a + // reference type, so a null Nullable arrives as a NULL wrapper — guard it to default(inner) + // (same null → default(T) contract as the CLR Nullable path above), else extraction NREs and + // the whole write batch is dropped. + if (TryGetSingleSlotField(t, out var slot0)) + { + var inner = NormalizeStorageValue(Expression.Field(access, slot0)); + if (!t.IsValueType) + return Expression.Condition( + Expression.ReferenceEqual(access, Expression.Constant(null, t)), + Expression.Default(inner.Type), + inner); + return inner; + } + return access; } + /// + /// True when is the portable representation of a + /// (or any single-field composite): a one-slot ISlots. Multi-slot composites (real composite + /// index keys) and non-Slots types return false so only the nullable wrapper is unwrapped. + /// + private static bool TryGetSingleSlotField(Type t, out FieldInfo slot0) + { + slot0 = null!; + if (!typeof(ISlots).IsAssignableFrom(t)) + return false; + if (t.GetField("Slot1") != null) // 2+ slots → genuine composite, leave intact + return false; + var f = t.GetField("Slot0"); + if (f == null) + return false; + slot0 = f; + return true; + } + + /// + /// If is the portable representation of a over a + /// value type (a single-slot Slots<T>), returns Nullable<T> — the CLR type + /// a remote client serialized the field value with (typeof(TField)). Lets the server keep the + /// RemoteFieldCodec symmetric instead of calling .PrimitiveType on the non-primitive wrapper. + /// + internal static bool TryGetPortableNullableType(Type t, out Type nullableType) + { + nullableType = null!; + if (!TryGetSingleSlotField(t, out var slot0)) + return false; + var inner = slot0.FieldType; + if (!inner.IsValueType || Nullable.GetUnderlyingType(inner) != null) + return false; + nullableType = typeof(Nullable<>).MakeGenericType(inner); + return true; + } + /// /// Builds a boxed-value normalizer for query inputs: a user-supplied field value of the declared /// member type (e.g. an enum literal, or a Nullable<DateTime>) is converted to the index's @@ -304,6 +366,12 @@ private static Expression NormalizeStorageValue(Expression access) /// internal static Func BuildValueNormalizer(Type rawType) { + // Portable Nullable raw type (single-slot Slots): remote query values arrive already + // flattened to the inner T (the client boxes/serializes the underlying value, never the + // wrapper), so normalize as the inner type. Casting the incoming T to Slots would throw. + if (TryGetSingleSlotField(rawType, out var slot0)) + return BuildValueNormalizer(slot0.FieldType); + if (NormalizeStorageType(rawType) == rawType) return v => v; diff --git a/src/CatDb/Database/Indexing/TableIndexManager.Query.cs b/src/CatDb/Database/Indexing/TableIndexManager.Query.cs index ca99d88..49e9bc5 100644 --- a/src/CatDb/Database/Indexing/TableIndexManager.Query.cs +++ b/src/CatDb/Database/Indexing/TableIndexManager.Query.cs @@ -137,7 +137,16 @@ internal Type GetMemberType(string member) { var slot = ResolveSlotIndices([member])[0]; var dt = _locator.RecordDataType; - return dt.IsPrimitive ? dt.PrimitiveType : dt[slot].PrimitiveType; + var slotDt = dt.IsPrimitive ? dt : dt[slot]; + if (slotDt.IsPrimitive) + return slotDt.PrimitiveType; + + // Non-primitive slot: on a portable/remote table a Nullable field is carried as a + // single-slot Slots. Reconstruct Nullable so RemoteFieldCodec decodes the value with the + // same CLR type the client encoded it with (typeof(TField)); otherwise `.PrimitiveType` throws + // "The type (T) is not primitive" on the wrapper. + var clr = DataTypeUtils.BuildType(slotDt); + return SlotAccessor.TryGetPortableNullableType(clr, out var nullable) ? nullable : clr; } /// CLR type of the primary key (for remote key-range decoding). diff --git a/src/CatDb/WaterfallTree/TypeEngine.cs b/src/CatDb/WaterfallTree/TypeEngine.cs index e12958f..2b452b7 100644 --- a/src/CatDb/WaterfallTree/TypeEngine.cs +++ b/src/CatDb/WaterfallTree/TypeEngine.cs @@ -22,12 +22,14 @@ private static TypeEngine Create(Type type) Persist = new DataPersist(type, null, AllowNull.OnlyMembers) }; - if (DataTypeUtils.IsAllPrimitive(type) || type == typeof(Guid)) + if (DataTypeUtils.IsAllComparable(type)) { descriptor.Comparer = new DataComparer(type); descriptor.EqualityComparer = new DataEqualityComparer(type); - if (type != typeof(Guid)) + // IndexerPersist only for pure primitives — Guid (bare or as a composite slot, + // e.g. a non-unique index key over a Guid-keyed table) has no DataIndexerPersist. + if (DataTypeUtils.IsAllPrimitive(type) && type != typeof(Guid)) descriptor.IndexerPersist = new DataIndexerPersist(type); } diff --git a/tests/CatDb.Tests/Database/GuidPkNonUniqueReproTests.cs b/tests/CatDb.Tests/Database/GuidPkNonUniqueReproTests.cs new file mode 100644 index 0000000..17a1d44 --- /dev/null +++ b/tests/CatDb.Tests/Database/GuidPkNonUniqueReproTests.cs @@ -0,0 +1,47 @@ +// Repro for reported: NonUnique index + Guid primary key -> NRE in OrderedSet.FindIndexes +using CatDb.Database; +using CatDb.Database.Indexing; +using CatDb.Extensions; +using FluentAssertions; + +namespace CatDb.Tests.Database; + +public class GuidPkNonUniqueReproTests : IDisposable +{ + private readonly IStorageEngine _engine = CatDb.Database.CatDb.FromMemory(); + public void Dispose() => _engine.Dispose(); + + public class Entity + { + public Guid AppId { get; set; } + public string City { get; set; } = ""; + public int Age { get; set; } + } + + [Fact] + public void GuidPk_NonUnique_StringField() + { + var t = _engine.OpenXTable("g_str"); + t.CreateIndex("City", e => e.City, IndexType.NonUnique); + for (int i = 0; i < 50; i++) + t.Replace(Guid.NewGuid(), new Entity { AppId = Guid.NewGuid(), City = i % 2 == 0 ? "NYC" : "LA", Age = i }); + _engine.Commit(); + + var res = t.Query(x => x.City).Equal("NYC").ToList(); + res.Should().HaveCount(25); + } + + [Fact] + public void GuidPk_NonUnique_GuidField() + { + var t = _engine.OpenXTable("g_guid"); + t.CreateIndex("AppId", e => e.AppId, IndexType.NonUnique); + var app = Guid.NewGuid(); + for (int i = 0; i < 20; i++) + t.Replace(Guid.NewGuid(), new Entity { AppId = i < 10 ? app : Guid.NewGuid(), City = "X", Age = i }); + _engine.Commit(); + + var res = t.Query(x => x.AppId).Equal(app).ToList(); + res.Should().HaveCount(10); + } +} diff --git a/tests/CatDb.Tests/Database/RemoteInsertReproTests.cs b/tests/CatDb.Tests/Database/RemoteInsertReproTests.cs new file mode 100644 index 0000000..1f84626 --- /dev/null +++ b/tests/CatDb.Tests/Database/RemoteInsertReproTests.cs @@ -0,0 +1,79 @@ +// PRE-EXISTING BUG (documented, not yet fixed) — remote/portable + Nullable secondary index. +// +// A remote table's records reach the server in its PORTABLE form (StorageEngineServer opens +// OpenXTablePortable(name, KeyDataType, RecordDataType) — record type built from the wire schema, +// NOT the CLR record type). A Nullable field transports as a nested Slots(T). The index layer's +// NormalizeStorageType only strips a CLR Nullable (Nullable.GetUnderlyingType), NOT the portable +// Slots, so a non-unique index over a nullable field builds a malformed composite key +// Slots(Slots, pk). Inserting through that index then SILENTLY drops most main-table records +// (no exception): after 100 remote Replace + Commit only ~2 rows survive server-side. +// +// Confirmed scope: non-nullable DateTime index works (100). Enum index works (normalizes to a flat +// integral). ONLY Nullable index fields lose records, independent of the values (null / non-null / +// distinct all fail). Embedded (typed) tables are unaffected. +// +// These tests are Skip-marked so the suite stays green; un-skip when fixing the portable-nullable path. +using System.Net; +using System.Net.Sockets; +using CatDb.Database; +using CatDb.Database.Indexing; +using CatDb.Remote; +using CatDb.General.Communication; +using FluentAssertions; + +namespace CatDb.Tests.Database; + +public class RemoteInsertReproTests +{ + private const string BugSkip = + "Pre-existing: remote/portable Nullable secondary index silently drops main-table records."; + + public enum St { A = 0, B = 1, C = 2 } + public class Rec { public Guid Id { get; set; } public St Status { get; set; } public DateTime? ExpiresAt { get; set; } } + public class Rec2 { public Guid Id { get; set; } public DateTime When { get; set; } } + + private static int FreePort() + { + var l = new TcpListener(IPAddress.Loopback, 0); l.Start(); + var p = ((IPEndPoint)l.LocalEndpoint).Port; l.Stop(); return p; + } + + private static async Task RemoteInsertCount(Func make, + Action>? createIndexes = null) + { + var port = FreePort(); + using var serverEngine = CatDb.Database.CatDb.FromMemory(); + await using var tcp = new TcpServer(port); + var server = new StorageEngineServer(serverEngine, tcp, accessPolicy: null); + await server.StartAsync(); + try + { + using var client = CatDb.Database.CatDb.FromNetwork("localhost", port, "default", "u", "p"); + var t = client.OpenXTable("t"); + createIndexes?.Invoke(t); + for (long i = 0; i < 100; i++) t.Replace(i, make(i)); + client.Commit(); + return t.Count(); + } + finally { await server.StopAsync(); } + } + + [Fact] + public async Task Remote_Insert_NoIndex_GuidEnumNullable() + => (await RemoteInsertCount(i => new Rec { Id = Guid.NewGuid(), Status = (St)(i % 3), ExpiresAt = i % 4 == 0 ? null : DateTime.UtcNow })).Should().Be(100); + + [Fact] + public async Task Remote_Insert_EnumIndex() + => (await RemoteInsertCount(i => new Rec { Id = Guid.NewGuid(), Status = (St)(i % 3), ExpiresAt = DateTime.UtcNow }, + t => t.CreateIndex("Status", r => r.Status, IndexType.NonUnique))).Should().Be(100); + + [Fact] + public async Task Remote_Insert_NonNullableDateTimeIndex() + => (await RemoteInsertCount(i => new Rec2 { Id = Guid.NewGuid(), When = new DateTime(2026, 1, 1).AddDays(i) }, + t => t.CreateIndex("When", r => r.When, IndexType.NonUnique))).Should().Be(100); + + [Fact] + public async Task Remote_Insert_NullableIndex() + => (await RemoteInsertCount(i => new Rec { Id = Guid.NewGuid(), Status = St.A, ExpiresAt = new DateTime(2026, 1, 1).AddDays(i) }, + t => t.CreateIndex("ExpiresAt", r => r.ExpiresAt, IndexType.NonUnique))).Should().Be(100); +} diff --git a/tests/CatDb.Tests/Database/RemoteNullableQueryTests.cs b/tests/CatDb.Tests/Database/RemoteNullableQueryTests.cs new file mode 100644 index 0000000..8dae840 --- /dev/null +++ b/tests/CatDb.Tests/Database/RemoteNullableQueryTests.cs @@ -0,0 +1,55 @@ +using System.Net; +using System.Net.Sockets; +using CatDb.Database; +using CatDb.Database.Indexing; +using CatDb.Extensions; +using CatDb.Remote; +using CatDb.General.Communication; +using FluentAssertions; + +namespace CatDb.Tests.Database; + +public class RemoteNullableQueryTests +{ + public class Item { public Guid Id { get; set; } public DateTime? ExpiresAt { get; set; } } + + private static int FreePort() + { + var l = new TcpListener(IPAddress.Loopback, 0); l.Start(); + var p = ((IPEndPoint)l.LocalEndpoint).Port; l.Stop(); return p; + } + + [Fact] + public async Task Remote_NullableIndex_DirectApi_And_Query() + { + var port = FreePort(); + using var serverEngine = CatDb.Database.CatDb.FromMemory(); + await using var tcp = new TcpServer(port); + var server = new StorageEngineServer(serverEngine, tcp, accessPolicy: null); + await server.StartAsync(); + try + { + using var client = CatDb.Database.CatDb.FromNetwork("localhost", port, "default", "u", "p"); + var t = client.OpenXTable("items"); + t.CreateIndex("ExpiresAt", r => r.ExpiresAt, IndexType.NonUnique); + + var baseTime = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + for (long i = 0; i < 100; i++) + t.Replace(i, new Item { Id = Guid.NewGuid(), ExpiresAt = i % 4 == 0 ? null : baseTime.AddDays(i) }); + client.Commit(); + + t.Count().Should().Be(100); + + // direct index API + var d5 = baseTime.AddDays(5); + t.Indexes.FindByIndex("ExpiresAt", d5).Count().Should().Be(1); + t.Indexes.CountByIndex("ExpiresAt", d5).Should().Be(1); + + // general query API (the path the stress test uses) + t.Query(x => x.ExpiresAt).Equal(d5).Count().Should().Be(1); + t.Query(x => x.ExpiresAt).AtLeast(baseTime.AddDays(10)).AtMost(baseTime.AddDays(50)).Count() + .Should().Be(Enumerable.Range(0, 100).Count(i => i % 4 != 0 && baseTime.AddDays(i) >= baseTime.AddDays(10) && baseTime.AddDays(i) <= baseTime.AddDays(50))); + } + finally { await server.StopAsync(); } + } +}