From 8bfa3934a0bdccf4d17a67d40bead8b586bb4242 Mon Sep 17 00:00:00 2001 From: MrDevRobot Date: Thu, 15 Oct 2026 20:54:46 +0200 Subject: [PATCH] fix(index): express comparison bounds in the indexed property's type (#222) IndexOptimizer.ParseSimplePredicate kept the runtime type of the constant in a comparison such as r.Count < 9.5 or r.Count >= 5L, where the compiler wraps the int property in a Convert to double or long. Index keys carry no type tag and compare bytewise, so a double or long bound against an int index was encoded in another key layout and selected the wrong range: Count(r => r.Count < 9.5) counted every document, Where(r => r.Count >= 2.5) returned none, and Max over such a filter could throw. The bound is now converted to the property's type when the property is widened by the comparison. For int and long properties an integer bound that selects the same values is chosen (Count < 9.5 becomes Count <= 9, Count >= 2.5 becomes Count >= 3, Count > 9.0 stays strict); a bound beyond the property's range, or NaN, turns the comparison into Count >= MinValue or Count > MaxValue, so plans stay exact for the count and aggregate push-downs and nulls of an int? property stay excluded. Pairs whose keys already share a layout (float/double, DateTime/DateTimeOffset, numeric bounds on a decimal-keyed index, enums) are kept. Anything else, such as a narrowing cast like (int)r.Total == 5 or a double bound beyond 2^53 on a long, plans no index, so the query evaluates the predicate per document. Only Convert/ConvertChecked unary nodes are treated as the property itself now, so Negate or ArrayLength over a property no longer becomes a key range. DynamicCollection does not use this planner and is unaffected. Co-Authored-By: Claude Opus 5.5 --- .../Collections/DocumentCollection.cs | 8 +- src/BLite.Core/Query/IndexOptimizer.cs | 134 +++++++++++- .../BLite.Tests/IndexBoundConversionTests.cs | 190 ++++++++++++++++++ tests/BLite.Tests/LinqAggregateTests.cs | 4 +- 4 files changed, 328 insertions(+), 8 deletions(-) create mode 100644 tests/BLite.Tests/IndexBoundConversionTests.cs diff --git a/src/BLite.Core/Collections/DocumentCollection.cs b/src/BLite.Core/Collections/DocumentCollection.cs index c417aa8..948f573 100644 --- a/src/BLite.Core/Collections/DocumentCollection.cs +++ b/src/BLite.Core/Collections/DocumentCollection.cs @@ -3775,10 +3775,10 @@ internal async Task CountByPredicateAsync( && plan.MinValue is null or TValue && plan.MaxValue is null or TValue) { - // The optimizer keeps the constant's own type (r.Count < 9.5 gives a double - // bound), and keys carry no type tag, so a bound of another type would be - // compared bytewise against the int/long keys. Such filters take the path below, - // which post-filters the documents the index returns. + // Keys carry no type tag, so a bound of another type would be compared bytewise + // against the int/long keys. The optimizer expresses bounds in the property's + // type (r.Count < 9.5 gives Count <= 9, #222); any other bound takes the path + // below, which post-filters the documents the index returns. fromKeys = AggregateIndexKeys(op, nullable, index, plan.MinValue, plan.MaxValue, plan.StartInclusive, plan.EndInclusive); } if (fromKeys.Handled) diff --git a/src/BLite.Core/Query/IndexOptimizer.cs b/src/BLite.Core/Query/IndexOptimizer.cs index 955d94d..d7ab7ea 100644 --- a/src/BLite.Core/Query/IndexOptimizer.cs +++ b/src/BLite.Core/Query/IndexOptimizer.cs @@ -715,7 +715,8 @@ private static (string? propertyName, object? value, ExpressionType op) ParseSim } // Handle Convert (e.g. enum property compared to int constant) - if (left is UnaryExpression unary && unary.Operand is MemberExpression member2 && right is ConstantExpression constant2) + if (left is UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked } unary + && unary.Operand is MemberExpression member2 && right is ConstantExpression constant2) { var propertyPath = ExtractMemberPath(member2, parameter); if (propertyPath != null) @@ -726,6 +727,8 @@ private static (string? propertyName, object? value, ExpressionType op) ParseSim var val2 = constant2.Value; if (propType.IsEnum && val2 != null && !val2.GetType().IsEnum) val2 = Enum.ToObject(propType, val2); + if (!HasConverter(propertyPath, registry) && !TryConvertBound(propType, unary.Type, ref val2, ref nodeType)) + return (null, null, ExpressionType.Default); return (propertyPath, val2, nodeType); } } @@ -748,7 +751,8 @@ private static (string? propertyName, object? value, ExpressionType op) ParseSim } // Handle Convert on left with closure on right - if (left is UnaryExpression unaryLeft && unaryLeft.Operand is MemberExpression memberLeft2) + if (left is UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked } unaryLeft + && unaryLeft.Operand is MemberExpression memberLeft2) { var propertyPath = ExtractMemberPath(memberLeft2, parameter); if (propertyPath != null) @@ -760,6 +764,8 @@ private static (string? propertyName, object? value, ExpressionType op) ParseSim var propType = unaryLeft.Operand.Type; if (propType.IsEnum && val != null && !val.GetType().IsEnum) val = Enum.ToObject(propType, val); + if (!HasConverter(propertyPath, registry) && !TryConvertBound(propType, unaryLeft.Type, ref val, ref nodeType)) + return (null, null, ExpressionType.Default); val = TryApplyConverter(propertyPath, val, registry); if (IsIndexableValue(val) || (val != null && val.GetType().IsEnum)) return (propertyPath, val, nodeType); @@ -929,6 +935,130 @@ private sealed class InValueComparer : IEqualityComparer return registry.TryConvert(topProp, value, out var pv) ? pv : value; } + private static bool HasConverter(string propertyPath, ValueConverterRegistry? registry) + { + if (registry == null) return false; + var topProp = propertyPath.Contains('.') ? propertyPath[..propertyPath.IndexOf('.')] : propertyPath; + return registry.HasConverter(topProp); + } + + /// + /// Expresses the bound of ()property op value in the + /// property's own type, adjusting where needed (#222). Index keys carry no + /// type tag and compare bytewise, so a bound of another type (r.Count < 9.5, + /// r.Count >= 5L on an int index) would be encoded in another key layout and + /// bound the wrong range. + /// + /// An int/long property gets the integer bound that selects the same values: + /// Count < 9.5 becomes Count <= 9, Count >= 2.5 becomes + /// Count >= 3. A bound beyond the property's range (or NaN) makes the comparison + /// always true, written Count >= int.MinValue (which still leaves out nulls, as + /// the lifted comparison does), or always false, written Count > int.MaxValue. + /// Bounds whose keys are already laid out like the property's are kept: float and + /// double (a float key is encoded as a double), DateTime and + /// DateTimeOffset (both as Unix milliseconds), and numeric bounds on a + /// decimal property, which the decimal-keyed index converts itself. + /// + /// Returns false when the bound has no exact equivalent in the property's type: a + /// narrowing cast ((int)r.Total == 5 also matches other values of Total), a + /// floating-point bound beyond the range where the property converts exactly (2^53 for + /// long to double, 2^24 to float), a NaN bound on a floating-point or + /// decimal property, or a pair of types not listed above. + /// The caller then plans no index for the comparison, and the query evaluates it per document. + /// + private static bool TryConvertBound(Type propertyType, Type comparisonType, ref object? value, ref ExpressionType op) + { + var target = Nullable.GetUnderlyingType(propertyType) ?? propertyType; + if (value == null || value is DBNull || value.GetType() == target) + return true; + if (op is not (ExpressionType.Equal or ExpressionType.GreaterThan or ExpressionType.GreaterThanOrEqual + or ExpressionType.LessThan or ExpressionType.LessThanOrEqual)) + return true; // not planned on an index anyway + + if (target.IsEnum) + { + // A nullable enum is compared through its underlying type: key it as the enum, like Insert. + if (value is not (int or long or short or byte or sbyte or ushort or uint)) return false; + value = Enum.ToObject(target, value); + return true; + } + + if (target != typeof(int) && target != typeof(long)) + { + // NaN has no place in key order: no comparison with it is true. + if (value is double dn && double.IsNaN(dn) || value is float fn && float.IsNaN(fn)) + return false; + if (target == typeof(double) || target == typeof(float)) + return value is double or float; + if (target == typeof(decimal)) + return value is int or long or double or float; + if (target == typeof(DateTime) || target == typeof(DateTimeOffset)) + return value is DateTime or DateTimeOffset; + return false; + } + + // Only a widening conversion of the property keeps a comparison equivalent to one bound on it; + // beyond `exactLimit` distinct property values convert to the same number. + var cmp = Nullable.GetUnderlyingType(comparisonType) ?? comparisonType; + double exactLimit; + if ((cmp == typeof(long) && target == typeof(int)) || cmp == typeof(decimal)) exactLimit = double.PositiveInfinity; + else if (cmp == typeof(double)) exactLimit = target == typeof(int) ? double.PositiveInfinity : 9007199254740992d; + else if (cmp == typeof(float)) exactLimit = 16777216d; + else return false; + + long min = target == typeof(int) ? int.MinValue : long.MinValue; + long max = target == typeof(int) ? int.MaxValue : long.MaxValue; + long floor = 0, ceiling = 0; + bool below = false, above = false, never = false; + switch (value) + { + case int i: + floor = ceiling = i; + break; + case long l: + if (l < min) below = true; + else if (l > max) above = true; + else floor = ceiling = l; + break; + case decimal m: + if (m < min) below = true; + else if (m > max) above = true; + else { floor = (long)decimal.Floor(m); ceiling = (long)decimal.Ceiling(m); } + break; + case double or float: + var d = Convert.ToDouble(value); + if (double.IsNaN(d)) { never = true; break; } + if (!double.IsPositiveInfinity(exactLimit) && !(Math.Abs(d) < exactLimit)) return false; + if (d < min) below = true; + else if (d > max) above = true; + else { floor = (long)Math.Floor(d); ceiling = (long)Math.Ceiling(d); } + break; + default: + return false; + } + + if (below || above || never) + { + bool always = !never && (op is ExpressionType.LessThan or ExpressionType.LessThanOrEqual ? above + : op is ExpressionType.GreaterThan or ExpressionType.GreaterThanOrEqual && below); + (floor, op) = always ? (min, ExpressionType.GreaterThanOrEqual) : (max, ExpressionType.GreaterThan); + } + else + { + bool integral = floor == ceiling; + switch (op) + { + case ExpressionType.GreaterThanOrEqual: floor = ceiling; break; + case ExpressionType.GreaterThan: if (!integral) { floor = ceiling; op = ExpressionType.GreaterThanOrEqual; } break; + case ExpressionType.LessThan: if (!integral) op = ExpressionType.LessThanOrEqual; break; + case ExpressionType.Equal: if (!integral) { floor = max; op = ExpressionType.GreaterThan; } break; + } + } + + value = target == typeof(int) ? (object)(int)floor : floor; + return true; + } + private static readonly HashSet _knownBsonPrimitives = [ typeof(int), typeof(long), typeof(double), typeof(decimal), diff --git a/tests/BLite.Tests/IndexBoundConversionTests.cs b/tests/BLite.Tests/IndexBoundConversionTests.cs new file mode 100644 index 0000000..320fd73 --- /dev/null +++ b/tests/BLite.Tests/IndexBoundConversionTests.cs @@ -0,0 +1,190 @@ +using System.Linq.Expressions; +using BLite.Bson; +using BLite.Core.Query; +using BLite.Shared; + +namespace BLite.Tests; + +/// +/// A comparison between an indexed property and a constant of another type (r.Count < 9.5, +/// r.Count >= 5L) selects the keys LINQ selects: index keys carry no type tag, so the bound +/// must be expressed in the property's type before it is used as a key (#222). +/// +public class IndexBoundConversionTests : IDisposable +{ + private readonly string _dbPath; + private readonly AggregateDbContext _db; + private readonly List _readings; + + public IndexBoundConversionTests() + { + _dbPath = Path.Combine(Path.GetTempPath(), $"index_bound_conversion_{Guid.NewGuid():N}.db"); + _db = new AggregateDbContext(_dbPath); + // Count (int), Total (long) and MaybeCount (int?) are indexed. The extremes of each type + // check that a bound outside the property's range keeps or drops every key. + _readings = Enumerable.Range(0, 20) + .Select(i => new Reading { Id = new ObjectId(i + 1, 0), Tag = $"r{i:00}", Count = i, Total = i, MaybeCount = i % 4 == 0 ? null : i }) + .Append(new Reading { Id = new ObjectId(21, 0), Tag = "min", Count = int.MinValue, Total = long.MinValue, MaybeCount = int.MinValue }) + .Append(new Reading { Id = new ObjectId(22, 0), Tag = "max", Count = int.MaxValue, Total = long.MaxValue, MaybeCount = int.MaxValue }) + .ToList(); + _db.Readings.InsertBulkAsync(_readings).GetAwaiter().GetResult(); + } + + public void Dispose() + { + _db.Dispose(); + foreach (var f in new[] { _dbPath, Path.ChangeExtension(_dbPath, ".wal") }) + try { if (File.Exists(f)) File.Delete(f); } catch { /* best-effort */ } + } + + private static double s_doubleBound = 9.5; + private static long s_longBound = 5L; + + public static TheoryData Predicates => new(s_predicates.Keys); + + private static readonly Dictionary>> s_predicates = new() + { + // From the issue. + ["int < double"] = r => r.Count < 9.5, + ["int >= long"] = r => r.Count >= 5L, + ["int <= long"] = r => r.Count <= 5L, + ["int >= double"] = r => r.Count >= 2.5, + + // int property, double bound. + ["int <= double"] = r => r.Count <= 9.5, + ["int > double"] = r => r.Count > 9.5, + ["int > integral double"] = r => r.Count > 9.0, + ["int < integral double"] = r => r.Count < 9.0, + ["int == integral double"] = r => r.Count == 5.0, + ["int == fractional double"] = r => r.Count == 5.5, + ["int < negative double"] = r => r.Count < -0.5, + ["int >= negative double"] = r => r.Count >= -0.5, + ["int < double above range"] = r => r.Count < 1e10, + ["int > double above range"] = r => r.Count > 1e10, + ["int >= double below range"] = r => r.Count >= -1e10, + ["int <= double below range"] = r => r.Count <= -1e10, + ["int == double above range"] = r => r.Count == 1e10, + ["int < NaN"] = r => r.Count < double.NaN, + ["int >= NaN"] = r => r.Count >= double.NaN, + ["int < +infinity"] = r => r.Count < double.PositiveInfinity, + ["int > -infinity"] = r => r.Count > double.NegativeInfinity, + ["int >= +infinity"] = r => r.Count >= double.PositiveInfinity, + ["int < captured double"] = r => r.Count < s_doubleBound, + ["double > int"] = r => 9.5 > r.Count, + + // int property, long / float / decimal bound. + ["int > long"] = r => r.Count > 5L, + ["int < long"] = r => r.Count < 5L, + ["int == long"] = r => r.Count == 5L, + ["int >= captured long"] = r => r.Count >= s_longBound, + ["int < long above range"] = r => r.Count < 5_000_000_000L, + ["int >= long above range"] = r => r.Count >= 5_000_000_000L, + ["int > long below range"] = r => r.Count > -5_000_000_000L, + ["int <= int.MaxValue as long"] = r => r.Count <= (long)int.MaxValue, + ["int > int.MaxValue as long"] = r => r.Count > (long)int.MaxValue, + ["int < float"] = r => r.Count < 9.5f, + ["int >= float"] = r => r.Count >= 2.5f, + ["int >= float beyond exact range"] = r => r.Count >= 1e9f, + ["int < decimal"] = r => r.Count < 9.5m, + ["int >= decimal"] = r => r.Count >= 2.5m, + ["int > decimal"] = r => r.Count > 9.5m, + ["int <= decimal above range"] = r => r.Count <= 1e20m, + + // long property. + ["long < double"] = r => r.Total < 9.5, + ["long >= double"] = r => r.Total >= 2.5, + ["long <= integral double"] = r => r.Total <= 5.0, + ["long > double beyond exact range"] = r => r.Total > 1e17, + ["long < double above range"] = r => r.Total < 1e19, + ["long >= decimal"] = r => r.Total >= 2.5m, + ["long < decimal above range"] = r => r.Total < 1e20m, + ["long < float"] = r => r.Total < 9.5f, + + // int? property. + ["int? < double"] = r => r.MaybeCount < 9.5, + ["int? >= long"] = r => r.MaybeCount >= 5L, + ["int? < double above range"] = r => r.MaybeCount < 1e10, + ["int? == fractional double"] = r => r.MaybeCount == 5.5, + + // AND of two converted bounds on one index. + ["double range"] = r => r.Count >= 2.5 && r.Count < 9.5, + ["int and double bounds"] = r => r.Count >= 2 && r.Count < 9.5, + ["double and long bounds"] = r => r.Count > 1.5 && r.Count <= 7L, + ["converted bound and other property"] = r => r.Count < 9.5 && r.Total >= 3L, + }; + + [Theory] + [MemberData(nameof(Predicates))] + public async Task Where_MatchesLinq(string name) + { + var predicate = s_predicates[name]; + var expected = _readings.Where(predicate.Compile()).Select(r => r.Tag).OrderBy(t => t).ToList(); + + var actual = (await _db.Readings.AsQueryable().Where(predicate).ToListAsync()).Select(r => r.Tag).OrderBy(t => t).ToList(); + + Assert.Equal(expected, actual); + } + + [Theory] + [MemberData(nameof(Predicates))] + public async Task CountWithPredicate_MatchesLinq(string name) + { + var predicate = s_predicates[name]; + + Assert.Equal(_readings.Count(predicate.Compile()), await _db.Readings.AsQueryable().CountAsync(predicate)); + } + + [Theory] + [MemberData(nameof(Predicates))] + public void Max_MatchesLinq(string name) + { + var predicate = s_predicates[name]; + var q = _db.Readings.AsQueryable(); + + Assert.Equal(Outcome(() => _readings.Where(predicate.Compile()).Max(r => r.Count)), Outcome(() => q.Where(predicate).Max(r => r.Count))); + Assert.Equal(Outcome(() => _readings.Where(predicate.Compile()).Max(r => r.Total)), Outcome(() => q.Where(predicate).Max(r => r.Total))); + Assert.Equal(Outcome(() => _readings.Where(predicate.Compile()).Min(r => r.MaybeCount)), Outcome(() => q.Where(predicate).Min(r => r.MaybeCount))); + } + + /// The value, or the type of the exception (Max over no element throws). + private static string Outcome(Func aggregate) + { + try { return $"value {aggregate()}"; } + catch (Exception ex) { return ex.GetType().Name; } + } + + [Fact] + public void Plan_FractionalStrictUpperBound_BecomesAnInclusiveIntBound() + { + var plan = IndexOptimizer.TryOptimize( + (Expression>)(r => r.Count < 9.5), _db.Readings.GetIndexes()); + + Assert.NotNull(plan); + Assert.Equal(9, plan!.MaxValue); + Assert.True(plan.EndInclusive); + Assert.Null(plan.MinValue); + Assert.Equal(IndexOptimizer.FilterCompleteness.Exact, plan.FilterCompleteness); + Assert.False(plan.HasResiduePredicate); + } + + [Fact] + public void Plan_LongBoundOnIntIndex_IsAnIntBound() + { + var plan = IndexOptimizer.TryOptimize( + (Expression>)(r => r.Count >= 5L), _db.Readings.GetIndexes()); + + Assert.NotNull(plan); + Assert.Equal(5, plan!.MinValue); + Assert.True(plan.StartInclusive); + } + + [Fact] + public void Plan_BoundThatCannotBeConverted_UsesNoIndex() + { + // Above 2^53 a long is not exactly a double: the bound has no exact long equivalent here. + var plan = IndexOptimizer.TryOptimize( + (Expression>)(r => r.Total > 1e17), _db.Readings.GetIndexes()); + + Assert.Null(plan); + } +} diff --git a/tests/BLite.Tests/LinqAggregateTests.cs b/tests/BLite.Tests/LinqAggregateTests.cs index 5522b62..649f15e 100644 --- a/tests/BLite.Tests/LinqAggregateTests.cs +++ b/tests/BLite.Tests/LinqAggregateTests.cs @@ -211,8 +211,8 @@ public async Task IndexOnAConvertedProperty_IsNotDecodedAsThePropertyType() [Fact] public async Task FilterBoundOfAnotherType_IsNotUsedAsAKeyRange() { - // The optimizer keeps the constant's type, so these bounds are a double and a long against - // the int Count index; encoded as keys they would bound the wrong range. + // These bounds are a double and a long against the int Count index; encoded as keys of + // their own type they would bound the wrong range (#222). using var db = new AggregateDbContext(_dbPath); for (int i = 0; i < 20; i++) await db.Readings.InsertAsync(new Reading { Id = new ObjectId(i + 1, 0), Count = i, Total = i });