diff --git a/src/BLite.Core/Collections/DocumentCollection.cs b/src/BLite.Core/Collections/DocumentCollection.cs index 95189124..f387bf12 100644 --- a/src/BLite.Core/Collections/DocumentCollection.cs +++ b/src/BLite.Core/Collections/DocumentCollection.cs @@ -902,7 +902,8 @@ internal DocumentCollection(StorageEngine storage, ITransactionHolder transactio /// /// Decimal properties used to be indexed as (double)value. An index whose persisted - /// predates the exact decimal encoding, or a full-text + /// predates the exact decimal encoding, a double-keyed + /// index holding a key of -0.0 from before -0.0 and +0.0 shared a key (#249), or a full-text /// index written in another format or holding keys it did not write, is rebuilt from the /// documents into a copy that queries don't see, the first time the collection is opened. Only /// after the copy committed is it swapped in, with its target format, in one metadata save, and diff --git a/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs b/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs index 887135d0..ed40ad25 100644 --- a/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs +++ b/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs @@ -805,18 +805,43 @@ private IndexKey ConvertToIndexKey(object value) private bool? _isDecimalKeyed; - private bool IsDecimalKeyed => _isDecimalKeyed ??= ComputeIsDecimalKeyed(); + private bool IsDecimalKeyed => _isDecimalKeyed ??= SingleKeyType() == typeof(decimal); - private bool ComputeIsDecimalKeyed() + /// The type of the single indexed property (nullable unwrapped), or null for a compound key. + private Type? SingleKeyType() { - if (_definition.PropertyPaths.Length != 1) return false; + if (_definition.PropertyPaths.Length != 1) return null; // Only the boxing conversion to object is looked through: an explicit cast in the selector // (e.g. (double)p.Price) changes the key type on purpose and is encoded as that type. var body = _definition.KeySelectorExpression.Body; while (body is System.Linq.Expressions.UnaryExpression { NodeType: System.Linq.Expressions.ExpressionType.Convert } u && u.Type == typeof(object)) body = u.Operand; - return (Nullable.GetUnderlyingType(body.Type) ?? body.Type) == typeof(decimal); + return Nullable.GetUnderlyingType(body.Type) ?? body.Type; + } + + // ── Double-keyed indexes ────────────────────────────────────────────────── + // IndexKey(double) gives -0.0 the key of +0.0 (#249). Earlier releases keyed -0.0 apart, just + // below +0.0; such an entry is neither found by a query nor removed when its document is updated + // or deleted, so a double- or float-keyed index that still holds one is rebuilt on open + // (NeedsKeyFormatMigration). Looking for it costs one seek per index and open. + + // The former key of -0.0: every bit of 0x8000_0000_0000_0000 inverted, after KeyPrefix. + private static readonly IndexKey s_legacyNegativeZeroKey = + new IndexKey(new byte[] { 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }); + + private bool HoldsLegacyNegativeZeroKey() + { + if (_btreeIndex == null) return false; + var keyType = SingleKeyType(); + if (keyType != typeof(double) && keyType != typeof(float)) return false; + + foreach (var _ in _btreeIndex.Range( + CreateCompositeKeyBoundary(s_legacyNegativeZeroKey, useMinObjectId: true), + CreateCompositeKeyBoundary(s_legacyNegativeZeroKey, useMinObjectId: false), + IndexDirection.Forward, null)) + return true; + return false; } private static decimal ToDecimalClamped(double d) @@ -848,10 +873,12 @@ private static decimal ToDecimalClamped(float f) /// True when this B-tree index keys a decimal property and its pages still use the legacy /// format, i.e. hold (double)value keys that the exact decimal encoding cannot match; or /// when this full-text index was written with another analyzer version or holds keys it did not - /// write (e.g. from an older BLite that read it as a plain B-tree). + /// write (e.g. from an older BLite that read it as a plain B-tree); or when this double- or + /// float-keyed B-tree index holds a key of -0.0 written before -0.0 and +0.0 shared a key. /// internal bool NeedsKeyFormatMigration => (_btreeIndex != null && KeyFormat < IndexMetadata.CurrentKeyFormat && IsDecimalKeyed) + || HoldsLegacyNegativeZeroKey() || (_fullTextIndex != null && (KeyFormat != FullTextIndex.FormatVersion || _fullTextIndex.HasForeignKeys())); /// The key format a rebuilt copy of this index is written in. diff --git a/src/BLite.Core/Indexing/IndexKey.cs b/src/BLite.Core/Indexing/IndexKey.cs index 35b2269f..a36926c0 100644 --- a/src/BLite.Core/Indexing/IndexKey.cs +++ b/src/BLite.Core/Indexing/IndexKey.cs @@ -97,6 +97,8 @@ public IndexKey(double value) // Positive: flip sign bit only → 1xxx… (positive doubles sort after all negatives) // Negative: flip all bits → sort from least-negative first // Result: lexicographic byte order == IEEE 754 numeric order for all finite and ±Inf. + // -0.0 == +0.0, so both take the key of +0.0; -0.0 would otherwise encode just below it (#249). + if (value == 0) value = 0.0; var bits = BitConverter.DoubleToInt64Bits(value); ulong u = bits < 0 ? ~(ulong)bits : (ulong)bits | 0x8000_0000_0000_0000UL; _data = [KeyPrefix, (byte)(u >> 56), (byte)(u >> 48), (byte)(u >> 40), (byte)(u >> 32), diff --git a/tests/BLite.Shared/SignedZeroContexts.cs b/tests/BLite.Shared/SignedZeroContexts.cs new file mode 100644 index 00000000..f1b16196 --- /dev/null +++ b/tests/BLite.Shared/SignedZeroContexts.cs @@ -0,0 +1,43 @@ +using BLite.Bson; +using BLite.Core; +using BLite.Core.Collections; +using BLite.Core.Metadata; + +namespace BLite.Shared; + +// Contexts for SignedZeroIndexTests (#249): the same collection with and without indexes on a +// double and a float property. + +public sealed class SzDoc +{ + public ObjectId Id { get; set; } + public string Name { get; set; } = ""; + public double Ratio { get; set; } + public float Level { get; set; } +} + +/// Indexes on and . +public partial class SzIndexedContext : DocumentDbContext +{ + public DocumentCollection Docs { get; set; } = null!; + public SzIndexedContext(string path) : base(path) => InitializeCollections(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().ToCollection("sz_docs") + .HasIndex(x => x.Ratio) + .HasIndex(x => x.Level); + } +} + +/// The same collection without secondary indexes. +public partial class SzPlainContext : DocumentDbContext +{ + public DocumentCollection Docs { get; set; } = null!; + public SzPlainContext(string path) : base(path) => InitializeCollections(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().ToCollection("sz_docs"); + } +} diff --git a/tests/BLite.Tests/SignedZeroIndexTests.cs b/tests/BLite.Tests/SignedZeroIndexTests.cs new file mode 100644 index 00000000..1bc82c79 --- /dev/null +++ b/tests/BLite.Tests/SignedZeroIndexTests.cs @@ -0,0 +1,202 @@ +using System.Linq.Expressions; +using BLite.Bson; +using BLite.Core.Indexing; +using BLite.Core.Query; +using BLite.Core.Storage; + +namespace BLite.Tests; + +/// +/// Regression tests for issue #249: IndexKey(double) gave -0.0 and +0.0 different keys, so +/// an index on a double missed -0.0 for == 0.0 and >= 0.0 and found it for +/// < 0.0, unlike IEEE 754 equality, which C# and the plain collection use. Each query must +/// agree with LINQ-to-Objects over the same documents, with and without the indexes. +/// +public class SignedZeroIndexTests : IAsyncLifetime +{ + private readonly string _indexedPath = Path.Combine(Path.GetTempPath(), $"sz_indexed_{Guid.NewGuid():N}.db"); + private readonly string _plainPath = Path.Combine(Path.GetTempPath(), $"sz_plain_{Guid.NewGuid():N}.db"); + private readonly string _legacyPath = Path.Combine(Path.GetTempPath(), $"sz_legacy_{Guid.NewGuid():N}.db"); + private SzIndexedContext _indexed = null!; + private SzPlainContext _plain = null!; + + private static SzDoc[] Seed() => + [ + new SzDoc { Name = "pz", Ratio = 0.0, Level = 0f }, + new SzDoc { Name = "a", Ratio = -2.5, Level = -2.5f }, + new SzDoc { Name = "nz", Ratio = -0.0, Level = -0f }, + new SzDoc { Name = "b", Ratio = 1.5, Level = 1.5f }, + new SzDoc { Name = "c", Ratio = -1.0, Level = -1f }, + new SzDoc { Name = "d", Ratio = 3.0, Level = 3f }, + ]; + + public async Task InitializeAsync() + { + _indexed = new SzIndexedContext(_indexedPath); + _plain = new SzPlainContext(_plainPath); + await _indexed.Docs.InsertBulkAsync(Seed()); + await _plain.Docs.InsertBulkAsync(Seed()); + } + + public Task DisposeAsync() + { + _indexed.Dispose(); + _plain.Dispose(); + foreach (var path in new[] { _indexedPath, _plainPath, _legacyPath }) + { + if (File.Exists(path)) File.Delete(path); + var wal = Path.ChangeExtension(path, ".wal"); + if (File.Exists(wal)) File.Delete(wal); + } + return Task.CompletedTask; + } + + [Fact] + public void IndexKey_Of_NegativeZero_Is_The_Key_Of_PositiveZero() + { + Assert.Equal(new IndexKey(0.0), new IndexKey(-0.0)); + Assert.True(new IndexKey(-double.Epsilon) < new IndexKey(-0.0)); + Assert.True(new IndexKey(-0.0) < new IndexKey(double.Epsilon)); + } + + [Fact] + public async Task Equality_Finds_Both_Zeros() + { + await AssertAgreesAsync(x => x.Ratio == 0.0, "nz", "pz"); + await AssertAgreesAsync(x => x.Ratio == -0.0, "nz", "pz"); + await AssertAgreesAsync(x => x.Level == 0f, "nz", "pz"); + } + + [Fact] + public async Task Ranges_Treat_Both_Zeros_Alike() + { + await AssertAgreesAsync(x => x.Ratio >= 0.0, "b", "d", "nz", "pz"); + await AssertAgreesAsync(x => x.Ratio > -0.0, "b", "d"); + await AssertAgreesAsync(x => x.Ratio < 0.0, "a", "c"); + await AssertAgreesAsync(x => x.Ratio <= -0.0, "a", "c", "nz", "pz"); + await AssertAgreesAsync(x => x.Ratio >= -1.0 && x.Ratio <= 0.0, "c", "nz", "pz"); + } + + [Fact] + public async Task Count_Finds_Both_Zeros() + { + foreach (var docs in new[] { _indexed.Docs, _plain.Docs }) + { + Assert.Equal(2, await docs.AsQueryable().CountAsync(x => x.Ratio == 0.0)); + Assert.Equal(4, await docs.AsQueryable().CountAsync(x => x.Ratio >= 0.0)); + Assert.Equal(2, await docs.AsQueryable().CountAsync(x => x.Ratio < 0.0)); + } + } + + [Fact] + public async Task OrderBy_Keeps_Both_Zeros_Together() + { + foreach (var (label, docs) in new[] { ("indexed", _indexed.Docs), ("plain", _plain.Docs) }) + { + var all = await docs.FindAllAsync().ToListAsync(); + + // The two zeros are equal, so their order within the window is not defined; the window + // over them holds both. + var window = (await docs.AsQueryable().OrderBy(x => x.Ratio).Skip(2).Take(2).ToListAsync()).Select(x => x.Name); + Assert.Equal(["nz", "pz"], window.Order(StringComparer.Ordinal)); + + var nonNegative = (await docs.AsQueryable().Where(x => x.Ratio >= 0.0).OrderBy(x => x.Ratio).ToListAsync()) + .Select(x => x.Ratio).ToArray(); + Assert.Equal(4, nonNegative.Length); + Assert.True(nonNegative.Zip(new[] { 0.0, 0.0, 1.5, 3.0 }).All(p => p.First == p.Second), label); + + var descending = (await docs.AsQueryable().OrderByDescending(x => x.Ratio).ToListAsync()).Select(x => x.Ratio).ToArray(); + Assert.True(descending.Zip(all.OrderByDescending(x => x.Ratio).Select(x => x.Ratio)).All(p => p.First == p.Second), label); + } + } + + /// + /// An index written before the fix keeps -0.0 under its former key, which the new bounds never + /// reach and which an update or delete of the document no longer removes. Opening the + /// collection rebuilds such an index; one without a -0.0 key is left alone. + /// + [Fact] + public async Task Index_Holding_The_Former_NegativeZero_Key_Is_Rebuilt_On_Open() + { + uint levelRoot; + using (var db = new SzIndexedContext(_legacyPath)) + { + await db.Docs.InsertBulkAsync(Seed()); + var index = RatioIndex(db); + var negativeZero = (await db.Docs.FindAllAsync().ToListAsync()).Single(x => x.Name == "nz"); + + // Recreate what an earlier release wrote for -0.0: [0x02 7F FF FF FF FF FF FF FF][id]. + var locations = index.BTreeIndex!.Range(IndexKey.NullSentinelNext, IndexKey.MaxKey).ToList(); + var current = locations.Single(e => e.Key.Data.StartsWith(new IndexKey(-0.0).Data) + && e.Key.Data.EndsWith(new IndexKey(negativeZero.Id).Data)); + var legacyUserKey = new byte[] { 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }; + var payload = legacyUserKey.Concat(new IndexKey(negativeZero.Id).Data.ToArray()).ToArray(); + using (var txn = db.BeginTransaction()) + { + Assert.True(index.BTreeIndex.Delete(current.Key, current.Location, txn.TransactionId)); + index.BTreeIndex.Insert(new IndexKey(payload), current.Location, txn.TransactionId); + await txn.CommitAsync(); + } + Assert.Contains(AllKeys(index), k => k.Data.StartsWith(new IndexKey(legacyUserKey).Data)); + levelRoot = LevelIndex(db).RootPageId; + } + + uint ratioRoot; + using (var db = new SzIndexedContext(_legacyPath)) + { + var index = RatioIndex(db); + Assert.DoesNotContain(AllKeys(index), k => k.Data[1] == 0x7F); + Assert.Equal(6, AllKeys(index).Count); + Assert.Equal(levelRoot, LevelIndex(db).RootPageId); + + Assert.Equal(["nz", "pz"], (await db.Docs.AsQueryable().Where(x => x.Ratio == 0.0).ToListAsync()) + .Select(x => x.Name).Order(StringComparer.Ordinal)); + + // The entry now goes away with its document. + var nz = (await db.Docs.FindAllAsync().ToListAsync()).Single(x => x.Name == "nz"); + Assert.True(await db.Docs.DeleteAsync(nz.Id)); + Assert.Equal(5, AllKeys(index).Count); + ratioRoot = index.RootPageId; + } + + using (var db = new SzIndexedContext(_legacyPath)) + { + // Nothing left to rebuild. + Assert.Equal(ratioRoot, RatioIndex(db).RootPageId); + Assert.Equal(levelRoot, LevelIndex(db).RootPageId); + Assert.Equal(["pz"], (await db.Docs.AsQueryable().Where(x => x.Ratio == 0.0).ToListAsync()).Select(x => x.Name)); + } + } + + // ─── Helpers ───────────────────────────────────────────────────────────── + + private static CollectionSecondaryIndex RatioIndex(SzIndexedContext db) + => db.Docs.IndexManager.GetAllIndexes().Single(i => i.PropertyPaths.Single() == nameof(SzDoc.Ratio)); + + private static CollectionSecondaryIndex LevelIndex(SzIndexedContext db) + => db.Docs.IndexManager.GetAllIndexes().Single(i => i.PropertyPaths.Single() == nameof(SzDoc.Level)); + + private static List AllKeys(CollectionSecondaryIndex index) + => index.BTreeIndex!.Range(IndexKey.NullSentinelNext, IndexKey.MaxKey).Select(e => e.Key).ToList(); + + /// + /// Runs on the indexed and the plain collection and checks both + /// against LINQ-to-Objects over the documents as read back, and against . + /// + private async Task AssertAgreesAsync(Expression> predicate, params string[] expected) + { + foreach (var (label, docs) in new[] { ("indexed", _indexed.Docs), ("plain", _plain.Docs) }) + { + var all = await docs.FindAllAsync().ToListAsync(); + var reference = Names(all.Where(predicate.Compile())); + Assert.Equal(expected, reference); + + var actual = Names(await docs.AsQueryable().Where(predicate).ToListAsync()); + Assert.True(reference.SequenceEqual(actual), + $"{label}: {predicate} returned [{string.Join(", ", actual)}], LINQ-to-Objects [{string.Join(", ", reference)}]"); + } + } + + private static string[] Names(IEnumerable docs) + => docs.Select(d => d.Name).Order(StringComparer.Ordinal).ToArray(); +}