diff --git a/src/BLite.Core/Collections/DocumentCollection.cs b/src/BLite.Core/Collections/DocumentCollection.cs index f387bf1..c95e8e8 100644 --- a/src/BLite.Core/Collections/DocumentCollection.cs +++ b/src/BLite.Core/Collections/DocumentCollection.cs @@ -903,7 +903,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, 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 holding a key of -0.0 from before -0.0 and +0.0 shared a key (#249) or of a NaN with the + /// sign bit clear from before every NaN shared one key (#257), 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 ed40ad2..00c4e7c 100644 --- a/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs +++ b/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs @@ -824,21 +824,36 @@ private IndexKey ConvertToIndexKey(object value) // 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. + // (NeedsKeyFormatMigration). The same holds for a NaN with the sign bit clear (what Arm64 + // computes), which was keyed above +Infinity before every NaN took the key of double.NaN. + // Looking for them costs two seeks 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() + // The former keys of the NaNs with the sign bit clear, 0x7FF0_0000_0000_0001..0x7FFF_FFFF_FFFF_FFFF + // with the sign bit set, after KeyPrefix: every key above that of +Infinity. + private static readonly IndexKey s_legacyPositiveNaNMinKey = + new IndexKey(new byte[] { 0xFF, 0xF0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 }); + private static readonly IndexKey s_legacyPositiveNaNMaxKey = + new IndexKey(new byte[] { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }); + + private bool HoldsLegacyDoubleKey() { 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), + return HoldsKeyBetween(s_legacyNegativeZeroKey, s_legacyNegativeZeroKey) + || HoldsKeyBetween(s_legacyPositiveNaNMinKey, s_legacyPositiveNaNMaxKey); + } + + private bool HoldsKeyBetween(IndexKey minUserKey, IndexKey maxUserKey) + { + foreach (var _ in _btreeIndex!.Range( + CreateCompositeKeyBoundary(minUserKey, useMinObjectId: true), + CreateCompositeKeyBoundary(maxUserKey, useMinObjectId: false), IndexDirection.Forward, null)) return true; return false; @@ -874,11 +889,12 @@ private static decimal ToDecimalClamped(float f) /// 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); 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. + /// float-keyed B-tree index holds a key of -0.0 written before -0.0 and +0.0 shared a key, or + /// a key of a NaN with the sign bit clear written before every NaN took the key of . /// internal bool NeedsKeyFormatMigration => (_btreeIndex != null && KeyFormat < IndexMetadata.CurrentKeyFormat && IsDecimalKeyed) - || HoldsLegacyNegativeZeroKey() + || HoldsLegacyDoubleKey() || (_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 a36926c..282dde9 100644 --- a/src/BLite.Core/Indexing/IndexKey.cs +++ b/src/BLite.Core/Indexing/IndexKey.cs @@ -99,7 +99,13 @@ public IndexKey(double value) // 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); + // A NaN's sign bit and payload are arbitrary (a computed NaN has the sign bit clear on Arm64, + // set on x64), which put NaNs both below -Inf and above +Inf. Every NaN takes one key, that of + // the bits of double.NaN (0xFFF8...), which sorts below -Inf, where double.CompareTo orders + // NaN (#257). The bits are spelled out so the key does not depend on how a NaN was produced. + var bits = double.IsNaN(value) + ? unchecked((long)0xFFF8_0000_0000_0000UL) + : 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), (byte)(u >> 24), (byte)(u >> 16), (byte)(u >> 8), (byte)u]; diff --git a/tests/BLite.Tests/IndexKeyNaNTests.cs b/tests/BLite.Tests/IndexKeyNaNTests.cs new file mode 100644 index 0000000..25998e8 --- /dev/null +++ b/tests/BLite.Tests/IndexKeyNaNTests.cs @@ -0,0 +1,141 @@ +using BLite.Bson; +using BLite.Core.Indexing; +using BLite.Core.Query; +using BLite.Core.Storage; + +namespace BLite.Tests; + +/// +/// Every NaN gets one index key, below -Infinity (#257). The sort-preserving double encoding keyed a +/// NaN by its bits, so a NaN with the sign bit clear (what Arm64 computes for 0.0 / 0.0) sorted above +/// +Infinity and one with the sign bit set (, what x64 computes) below +/// -Infinity: the order of an index-served query depended on the CPU that wrote the value. +/// +public sealed class IndexKeyNaNTests : IDisposable +{ + private static readonly double PositiveNaN = BitConverter.Int64BitsToDouble(0x7FF8_0000_0000_0000L); + private static readonly double PayloadNaN = BitConverter.Int64BitsToDouble(0x7FF0_0000_0000_0001L); + private static readonly double NegativePayloadNaN = BitConverter.Int64BitsToDouble(unchecked((long)0xFFF0_0000_0000_0001UL)); + + private readonly string _dbPath = Path.Combine(Path.GetTempPath(), $"blite_nan_{Guid.NewGuid():N}.db"); + + public void Dispose() + { + foreach (var p in new[] { _dbPath, Path.ChangeExtension(_dbPath, ".wal") }) + try { if (File.Exists(p)) File.Delete(p); } catch { } + } + + public static IEnumerable NaNs() => new[] + { + new object[] { PositiveNaN }, + new object[] { PayloadNaN }, + new object[] { NegativePayloadNaN }, + new object[] { double.NaN }, + }; + + [Theory] + [MemberData(nameof(NaNs))] + public void Every_NaN_Has_The_Key_Of_Double_NaN_Below_Negative_Infinity(double nan) + { + Assert.True(double.IsNaN(nan)); + var key = IndexKey.Create(nan); + + Assert.Equal(IndexKey.Create(double.NaN), key); + Assert.True(key < IndexKey.Create(double.NegativeInfinity)); + Assert.True(key < IndexKey.Create(double.MinValue)); + Assert.True(double.IsNaN(key.As())); + } + + [Fact] + public void Index_Orders_NaNs_First_And_Excludes_Them_From_A_Lower_Bounded_Range() + { + using var storage = new StorageEngine(_dbPath, PageFileConfig.Default); + var index = new BTreeIndex(storage, IndexOptions.CreateBTree("ratio")); + var txn = storage.BeginTransaction().TransactionId; + + index.Insert(IndexKey.Create(PositiveNaN), new DocumentLocation(1, 0), txn); // d1 + index.Insert(IndexKey.Create(1.0), new DocumentLocation(2, 0), txn); // d2 + index.Insert(IndexKey.Create(double.NaN), new DocumentLocation(3, 0), txn); // d3 + index.Insert(IndexKey.Create(double.PositiveInfinity), new DocumentLocation(4, 0), txn); + + var ascending = index.Range(IndexKey.MinKey, IndexKey.MaxKey, IndexDirection.Forward, txn) + .Select(e => e.Location.PageId).ToList(); + // The two NaNs share a key and keep their insertion order between themselves. + Assert.Equal(new uint[] { 1, 3 }, ascending.Take(2).OrderBy(p => p)); + Assert.Equal(new uint[] { 2, 4 }, ascending.Skip(2)); + + var aboveHalf = index.Range(IndexKey.Create(0.5), IndexKey.MaxKey, IndexDirection.Forward, txn) + .Select(e => e.Location.PageId).ToList(); + Assert.Equal(new uint[] { 2, 4 }, aboveHalf); + + var atLeastMax = index.Range(IndexKey.Create(double.MaxValue), IndexKey.MaxKey, IndexDirection.Forward, txn) + .Select(e => e.Location.PageId).ToList(); + Assert.Equal(new uint[] { 4 }, atLeastMax); + } + + /// + /// An index written before the fix keeps a NaN with the sign bit clear under its former key, + /// above +Infinity, which an update or delete of the document no longer removes. Opening the + /// collection rebuilds such an index; one holding +Infinity but no former NaN key is left alone. + /// + [Fact] + public async Task Index_Holding_The_Former_Positive_NaN_Key_Is_Rebuilt_On_Open() + { + var seed = new[] + { + new SzDoc { Name = "nan", Ratio = PositiveNaN }, + new SzDoc { Name = "one", Ratio = 1.0 }, + new SzDoc { Name = "inf", Ratio = double.PositiveInfinity }, + }; + // What an earlier release wrote for PositiveNaN: its bits with the sign bit set. + var legacyUserKey = new byte[] { 0xFF, 0xF8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; + + using (var db = new SzIndexedContext(_dbPath)) + { + await db.Docs.InsertBulkAsync(seed); + var index = RatioIndex(db); + var nan = (await db.Docs.FindAllAsync().ToListAsync()).Single(x => x.Name == "nan"); + + var current = index.BTreeIndex!.Range(IndexKey.NullSentinelNext, IndexKey.MaxKey).ToList() + .Single(e => e.Key.Data.StartsWith(new IndexKey(PositiveNaN).Data) + && e.Key.Data.EndsWith(new IndexKey(nan.Id).Data)); + var payload = legacyUserKey.Concat(new IndexKey(nan.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)); + } + + uint ratioRoot; + using (var db = new SzIndexedContext(_dbPath)) + { + var index = RatioIndex(db); + Assert.DoesNotContain(AllKeys(index), k => k.Data.StartsWith(new IndexKey(legacyUserKey).Data)); + Assert.Single(AllKeys(index), k => k.Data.StartsWith(new IndexKey(double.NaN).Data)); + Assert.Equal(3, AllKeys(index).Count); + + // The entry now goes away with its document. + var nan = (await db.Docs.FindAllAsync().ToListAsync()).Single(x => x.Name == "nan"); + Assert.True(await db.Docs.DeleteAsync(nan.Id)); + Assert.Equal(2, AllKeys(index).Count); + ratioRoot = index.RootPageId; + } + + using (var db = new SzIndexedContext(_dbPath)) + { + // The +Infinity key left is not a former NaN key: nothing left to rebuild. + Assert.Equal(ratioRoot, RatioIndex(db).RootPageId); + Assert.Equal(["inf", "one"], (await db.Docs.AsQueryable().Where(x => x.Ratio >= 1.0).ToListAsync()) + .Select(x => x.Name).Order(StringComparer.Ordinal)); + } + } + + private static CollectionSecondaryIndex RatioIndex(SzIndexedContext db) + => db.Docs.IndexManager.GetAllIndexes().Single(i => i.PropertyPaths.Single() == nameof(SzDoc.Ratio)); + + private static List AllKeys(CollectionSecondaryIndex index) + => index.BTreeIndex!.Range(IndexKey.NullSentinelNext, IndexKey.MaxKey).Select(e => e.Key).ToList(); +}