From 89a7a3814f4534698624a925e9514f288444478e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Sat, 24 Oct 2026 09:08:09 +0200 Subject: [PATCH] fix: declare lower() and upper() NULL for a non-null range (#349) BoundFunction declared lower() and upper() with argumentsPropagateNullability [true], which tells EF Core the bound is NULL only when the column is. PostgreSQL also returns NULL for the empty range and for an unbounded side, so EF Core folded LowerBound() == null on a required column to WHERE FALSE, dropped the COALESCE of a ??, and tested a nullable column's bound with the column's own IS NULL. The flag is now [false] for both, on ranges and on RangeSet. The bound sweep in ExecutedQueryTests now runs every operation on the RangeSet column of each family too (13 + 13 columns), and its nine #349 candidates are removed. With the old flag it fails on 234 rows, 117 per kind; with the new one every row agrees. Two translation tests pin the SQL without a database. Claim E70 covers RangeSet's bounds and quotes the efcore.md sentence that now promises the query answer. Found-By: US Defect-Class: SQL Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/efcore.md | 4 +- docs/migration.md | 43 +++++ .../CHANGELOG.md | 1 + .../CHANGELOG.md | 1 + .../Query/ValueRangesMethodCallTranslator.cs | 7 +- .../ExecutedQueryTests.cs | 170 ++++++++++-------- .../QueryTranslationTests.cs | 22 +++ test/oracles/candidates.json | 63 ------- test/oracles/claims.json | 8 +- 10 files changed, 176 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bac961bd..93b4649a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11. Not - All four packages target `net11.0` and need the .NET 11 runtime, one target framework per line; the EF Core packages reference EF Core 11 and `Npgsql.EntityFrameworkCore.PostgreSQL` 11 (release candidates while .NET 11 is in preview). PostgreSQL 16 or later, as in 10.x. - EF Core: `EFCore.ComplexIndexes.PostgreSQL` 5.x stays on EF Core 10 and can't be installed beside the 11 packages. Its EF Core 11 successor, `CodoMetis.EFCore.SchemaKit.PostgreSQL`, isn't on NuGet yet ([CaffeinatedCoder/EFCore.ComplexIndexes#38](https://github.com/CaffeinatedCoder/EFCore.ComplexIndexes/issues/38)). For exclusion constraints until then, stay on 10.x or add the constraint with explicit SQL. Temporal keys are Npgsql's own on EF Core 11 ([Exclusion constraints](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/efcore.md#reject-overlapping-rows-with-an-exclusion-constraint)). +- EF Core: `LowerBound()` and `UpperBound()` in a query are `NULL` for an empty range or set and on an unbounded side, as in memory. So `== null` selects those rows, where it selected none on a required column, and `?? fallback` returns the fallback, where the row was missing ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-bound-comparisons-with-null-in-queries)). ### Added diff --git a/docs/efcore.md b/docs/efcore.md index 69b137ca..99938016 100644 --- a/docs/efcore.md +++ b/docs/efcore.md @@ -147,7 +147,9 @@ Good to know: have a gap. PostgreSQL normalizes multiranges the same way this library does, so the equality is exact ([verified against live PostgreSQL](../README.md#verified-against-postgresql)). - **`LowerBound()` and `UpperBound()` return `T?`.** PostgreSQL's `lower` and `upper` return `NULL` - for an unbounded or empty range, and the in-memory methods match that. + for an unbounded or empty range, and the in-memory methods match that. So `== null`, `HasValue` + and `??` on a bound select the same rows in a query as in memory, on a range or a `RangeSet` + column. - **Discrete upper bounds are adjusted for you.** For `int4range`, `int8range` and `daterange`, PostgreSQL normalizes to the half-open form `[lower, upper)`. This library normalizes to the closed form `[lower, upper]`. So `UpperBound()` becomes `upper(x) - 1`, and diff --git a/docs/migration.md b/docs/migration.md index 0a604fb6..69bf7ac5 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -785,3 +785,46 @@ runtimes. Look at the text instead: - **Say "the end of the day" another way.** Use an unbounded end, `[22:00,)`, or an inclusive `23:59:59.9999999` bound ([TimeRange and the custom timerange type](efcore.md#timerange-and-the-custom-timerange-type)). + +## Migration from v10.x + +Nothing was removed or renamed. Some queries return different rows, and the compiler can't show you +where, so check these: + +- **EF Core: a bound tested for `null` in a query selects the rows memory selects.** + `LowerBound() == null` selected no rows of a required column, and `?? fallback` lost its fallback + ([details](#check-bound-comparisons-with-null-in-queries)). + +### Check bound comparisons with `null` in queries + +Check EF Core queries that call `LowerBound()` or `UpperBound()` on a range or `RangeSet` column and +then test the result for `null`. That covers `== null`, `!= null`, `HasValue` and `??` with a +fallback, in a filter or a projection. It also covers a filter on a bound you projected first, which +is what a data grid sends for a "blank" filter. + +PostgreSQL's `lower` and `upper` return `NULL` for an empty range and for an unbounded side. The +in-memory methods return `null` for the same values. Before 11.0.0, the EF Core package declared +that a bound is `NULL` only when the column is, and EF Core simplified the query on that basis. On a +required column: + +| Query | Before 11.0.0 | Now | +|---|---|---| +| `Where(r => r.Period.LowerBound() == null)` | no rows | the empty rows and the rows with no lower bound | +| `Where(r => r.Period.UpperBound() != null)`, or `HasValue` | every row | the rows with an upper bound | +| `Where(r => (r.Period.UpperBound() ?? DateOnly.MaxValue) >= day)` | the empty rows and the rows with no upper bound are missing | those rows match through the fallback | +| `Select(r => r.Period.UpperBound() ?? DateOnly.MaxValue)` | throws (`InvalidOperationException`) at an empty row or a row with no upper bound | returns the fallback for it | +| `Select(r => new { To = r.Period.UpperBound() }).Where(x => x.To == null)` | no rows | the empty rows and the rows with no upper bound | + +On a nullable column, `LowerBound() == null` ran as the column's own `IS NULL`. It found the `NULL` +rows and missed the empty and unbounded ones. + +This holds for every range type and for `RangeSet`: the .NET and NodaTime ranges, `TimeRange`, the +float ranges and `YearMonthRange`. Code that runs in memory is unchanged. + +- **Search your queries for `LowerBound()` and `UpperBound()`.** Each one followed by `== null`, + `!= null`, `HasValue` or `??` now returns the rows the same code returns in memory. +- **Keep or drop the workarounds you wrote for it.** A query that adds `IsEmpty()`, + `IsUnboundedStart()` or `IsUnboundedEnd()` beside the `null` test returns the same rows as before, + so the extra test is now optional. +- **Check reports built on the old rows.** On a required column, a count of rows with no end date + was 0, and a filter on a date with a `??` fallback left out every open-ended row. diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md index f7ef06c2..8bfe6eda 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md @@ -10,6 +10,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11, EF ### Breaking - Targets `net11.0` and references `Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime` 11 (a release candidate while .NET 11 is in preview). PostgreSQL 16 or later, as in 10.x. +- EF Core: `LowerBound()` and `UpperBound()` of a NodaTime range or multirange column, `YearMonthRange` included, are `NULL` in a query for an empty value and on an unbounded side, as in memory. So `== null` selects those rows, where it selected none on a required column, and `?? fallback` returns the fallback, where the row was missing ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-bound-comparisons-with-null-in-queries)). ### Added diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md index da2af891..b794890a 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md @@ -11,6 +11,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11, EF - Targets `net11.0` and references EF Core 11 and `Npgsql.EntityFrameworkCore.PostgreSQL` 11 (release candidates while .NET 11 is in preview). PostgreSQL 16 or later, as in 10.x. - EF Core: `EFCore.ComplexIndexes.PostgreSQL` 5.x stays on EF Core 10 and can't be installed beside the 11 packages. Its EF Core 11 successor, `CodoMetis.EFCore.SchemaKit.PostgreSQL`, isn't on NuGet yet ([CaffeinatedCoder/EFCore.ComplexIndexes#38](https://github.com/CaffeinatedCoder/EFCore.ComplexIndexes/issues/38)). For exclusion constraints until then, stay on 10.x or add the constraint with explicit SQL. Temporal keys are Npgsql's own on EF Core 11 ([Exclusion constraints](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/efcore.md#reject-overlapping-rows-with-an-exclusion-constraint)). +- EF Core: `LowerBound()` and `UpperBound()` in a query are `NULL` for an empty range or set and on an unbounded side, as in memory. So `== null` selects those rows, where it selected none on a required column, and `?? fallback` returns the fallback, where the row was missing ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-bound-comparisons-with-null-in-queries)). ### Added diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs index 95436f9c..fe6341cc 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs @@ -634,14 +634,17 @@ private SqlExpression BoolFunction(string name, SqlExpression argument) /// /// A bound-extraction function (lower/upper) on a range or multirange. /// PostgreSQL returns NULL for an empty or correspondingly unbounded operand, hence the - /// nullable CLR type. + /// nullable CLR type. That NULL comes from a non-null argument, so the argument must not be + /// declared as propagating nullability: EF Core would then take the bound of a required column + /// as never NULL, fold IS NULL to FALSE and drop the COALESCE of a + /// ??, and test a nullable column's bound with the column's own IS NULL. /// private SqlExpression BoundFunction(string name, SqlExpression argument, IRangeTypeDefinition definition) => sqlExpressionFactory.Function( name, [argument], nullable: true, - argumentsPropagateNullability: [true], + argumentsPropagateNullability: [false], typeof(Nullable<>).MakeGenericType(definition.ElementClrType), ElementMapping(definition)); diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/ExecutedQueryTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/ExecutedQueryTests.cs index 46e485dc..4455f43f 100644 --- a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/ExecutedQueryTests.cs +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/ExecutedQueryTests.cs @@ -636,11 +636,11 @@ await Seed( /// /// LowerBound() and UpperBound() compared with , read through /// HasValue, coalesced with ?? in a filter and a projection, and filtered after being - /// projected, for a range column of every family, against the same expression over the rows as - /// loaded. The bounds are NULL for the empty range and for the unbounded side of a non-null one, so - /// these forms only agree while EF Core knows lower and upper can be NULL for a non-null - /// argument (#349). Each failure line starts with the operation, then the column. Ids run from 2400 - /// to 2659, twenty per family. + /// projected, for a range column and a multirange column of every family, against the same + /// expression over the rows as loaded. The bounds are NULL for the empty range and for the unbounded + /// side of a non-null one, so these forms only agree while EF Core knows lower and + /// upper can be NULL for a non-null argument (#349). Each failure line starts with the + /// operation, then the column. Ids run from 2400 to 2659, twenty per family. /// [TestMethod] public async Task BoundsComparedWithNull_SelectTheRowsMemorySelects() @@ -652,25 +652,29 @@ public async Task BoundsComparedWithNull_SelectTheRowsMemorySelects() LocalDate Day(int day) => new(2026, 3, day); var failures = new List(); - failures.AddRange(await BoundNullSweep(2400, r => r.Seats, 1, 10, 20, int.MinValue, int.MaxValue)); - failures.AddRange(await BoundNullSweep(2420, r => r.Tickets, 1L, 10L, 20L, long.MinValue, long.MaxValue)); - failures.AddRange(await BoundNullSweep(2440, r => r.Price, 1.5m, 10m, 20.25m, decimal.MinValue, decimal.MaxValue)); - failures.AddRange(await BoundNullSweep(2460, r => r.Period, new DateOnly(2026, 3, 1), new DateOnly(2026, 3, 15), + failures.AddRange(await BoundNullSweep(2400, r => r.Seats, r => r.SeatBlocks, 1, 10, 20, int.MinValue, int.MaxValue)); + failures.AddRange(await BoundNullSweep(2420, r => r.Tickets, r => r.TicketBlocks, 1L, 10L, 20L, long.MinValue, long.MaxValue)); + failures.AddRange(await BoundNullSweep(2440, r => r.Price, r => r.PriceBands, 1.5m, 10m, 20.25m, decimal.MinValue, decimal.MaxValue)); + failures.AddRange(await BoundNullSweep(2460, r => r.Period, r => r.BlockedDays, new DateOnly(2026, 3, 1), new DateOnly(2026, 3, 15), new DateOnly(2026, 3, 31), DateOnly.MinValue, DateOnly.MaxValue)); - failures.AddRange(await BoundNullSweep(2480, r => r.LocalTime, at, at.AddDays(1), at.AddDays(2), DateTime.MinValue, DateTime.MaxValue)); - failures.AddRange(await BoundNullSweep(2500, r => r.InstantTime, utc, utc.AddDays(1), utc.AddDays(2), + failures.AddRange(await BoundNullSweep(2480, r => r.LocalTime, r => r.WallClockWindows, at, at.AddDays(1), at.AddDays(2), + DateTime.MinValue, DateTime.MaxValue)); + failures.AddRange(await BoundNullSweep(2500, r => r.InstantTime, r => r.InstantWindows, utc, utc.AddDays(1), utc.AddDays(2), DateTimeOffset.MinValue, DateTimeOffset.MaxValue)); - failures.AddRange(await BoundNullSweep(2520, r => r.NodaPeriod, Day(1), Day(15), Day(31), LocalDate.MinIsoValue, LocalDate.MaxIsoValue)); - failures.AddRange(await BoundNullSweep(2540, r => r.NodaWallClock, LocalDateTime.FromDateTime(at), LocalDateTime.FromDateTime(at.AddDays(1)), - LocalDateTime.FromDateTime(at.AddDays(2)), LocalDateTime.MinIsoValue, LocalDateTime.MaxIsoValue)); - failures.AddRange(await BoundNullSweep(2560, r => r.NodaWindow, Instant.FromDateTimeOffset(utc), Instant.FromDateTimeOffset(utc.AddDays(1)), - Instant.FromDateTimeOffset(utc.AddDays(2)), Instant.MinValue, Instant.MaxValue)); + failures.AddRange(await BoundNullSweep(2520, r => r.NodaPeriod, r => r.NodaBlockedDays, Day(1), Day(15), Day(31), + LocalDate.MinIsoValue, LocalDate.MaxIsoValue)); + failures.AddRange(await BoundNullSweep(2540, r => r.NodaWallClock, r => r.NodaWallClocks, LocalDateTime.FromDateTime(at), + LocalDateTime.FromDateTime(at.AddDays(1)), LocalDateTime.FromDateTime(at.AddDays(2)), + LocalDateTime.MinIsoValue, LocalDateTime.MaxIsoValue)); + failures.AddRange(await BoundNullSweep(2560, r => r.NodaWindow, r => r.NodaWindows, Instant.FromDateTimeOffset(utc), + Instant.FromDateTimeOffset(utc.AddDays(1)), Instant.FromDateTimeOffset(utc.AddDays(2)), + Instant.MinValue, Instant.MaxValue)); // time is microsecond-precise, so TimeOnly.MaxValue would round to 24:00; any fallback the column holds will do. - failures.AddRange(await BoundNullSweep(2580, r => r.OpeningHours, new TimeOnly(9, 0), new TimeOnly(12, 0), new TimeOnly(17, 0), - TimeOnly.MinValue, new TimeOnly(23, 59, 59))); - failures.AddRange(await BoundNullSweep(2600, r => r.Score, 1.5, 10.0, 20.25, double.MinValue, double.MaxValue)); - failures.AddRange(await BoundNullSweep(2620, r => r.Load, 1.5f, 10f, 20.25f, float.MinValue, float.MaxValue)); - failures.AddRange(await BoundNullSweep(2640, r => r.BillingPeriod, new YearMonth(2026, 1), new YearMonth(2026, 6), + failures.AddRange(await BoundNullSweep(2580, r => r.OpeningHours, r => r.OpeningWindows, new TimeOnly(9, 0), new TimeOnly(12, 0), + new TimeOnly(17, 0), TimeOnly.MinValue, new TimeOnly(23, 59, 59))); + failures.AddRange(await BoundNullSweep(2600, r => r.Score, r => r.ScoreBands, 1.5, 10.0, 20.25, double.MinValue, double.MaxValue)); + failures.AddRange(await BoundNullSweep(2620, r => r.Load, r => r.LoadBands, 1.5f, 10f, 20.25f, float.MinValue, float.MaxValue)); + failures.AddRange(await BoundNullSweep(2640, r => r.BillingPeriod, r => r.BillingPeriods, new YearMonth(2026, 1), new YearMonth(2026, 6), new YearMonth(2026, 12), new YearMonth(1, 1), new YearMonth(9999, 12))); Candidates.Verdict(nameof(ExecutedQueryTests), failures, @@ -679,14 +683,16 @@ public async Task BoundsComparedWithNull_SelectTheRowsMemorySelects() } /// - /// Seeds one row per shape into — finite across, below and above - /// , unbounded on either side of it, infinite and empty — and runs every bound - /// operation on the server and over the rows as loaded. and + /// Seeds one row per shape into , and the same shape as a one-element + /// multirange into — finite across, below and above , + /// unbounded on either side of it, infinite and empty — and runs every bound operation on both + /// columns, on the server and over the rows as loaded. and /// are the ?? fallbacks; they must round-trip through the column's /// element type, or the projection disagrees for that reason alone. /// private static async Task> BoundNullSweep( - int firstId, Expression> column, T a, T v, T b, T min, T max) + int firstId, Expression> column, Expression>> set, + T a, T v, T b, T min, T max) where TRange : class, IRangeFactory, IRange where T : struct, IComparable, IEquatable { @@ -703,77 +709,91 @@ private static async Task> BoundNullSweep( ("empty", TRange.Empty) ]; - var property = (PropertyInfo)((MemberExpression)column.Body).Member; - var names = new Dictionary(); - var rows = new List(); + var property = (PropertyInfo)((MemberExpression)column.Body).Member; + var setProperty = (PropertyInfo)((MemberExpression)set.Body).Member; + var names = new Dictionary(); + var rows = new List(); foreach (var (shape, index) in shapes.Select((shape, index) => (shape, index))) { var row = new Reservation { Id = firstId + index }; property.SetValue(row, shape.Range); + setProperty.SetValue(row, RangeSet.From(shape.Range)); rows.Add(row); names[row.Id] = shape.Name; } await Seed([.. rows]); - var row0 = column.Parameters[0]; - var label = $"{property.Name} ({typeof(TRange).Name})"; - var lastId = firstId + shapes.Length - 1; + var row0 = column.Parameters[0]; + var setColumn = Expression.Property(row0, setProperty); + var lastId = firstId + shapes.Length - 1; - // The operation written over the range as IRange, spliced onto the column, so the call is - // the one a consumer's r.Column.LowerBound() makes. - Expression> On(Expression, TOut>> operation) => - Expression.Lambda>(FuzzEngine.Splice(operation, row0, column.Body), row0); + await using var context = new IntegrationDbContext(); + var mine = context.Reservations.Where(r => r.Id >= firstId && r.Id <= lastId); + var loaded = await mine.AsNoTracking().OrderBy(r => r.Id).ToListAsync(); + var failures = new List(); + + // The range's bounds are the IRange extensions and the set's its own methods, each spliced + // onto its column, so the call is the one a consumer's r.Column.LowerBound() makes. + await Sweep($"{property.Name} ({typeof(TRange).Name})", + Bound>(column.Body, (_, x) => x.LowerBound()), Bound>(column.Body, (_, x) => x.UpperBound())); + await Sweep($"{setProperty.Name} (RangeSet<{typeof(TRange).Name}>)", + Bound>(setColumn, (_, x) => x.LowerBound()), Bound>(setColumn, (_, x) => x.UpperBound())); + + return failures; + + Expression> Bound(Expression on, Expression> bound) => + Expression.Lambda>(FuzzEngine.Splice(bound, row0, on), row0); + + // The operation written over the bound, spliced onto it. + Expression> Over(Expression> bound, Expression> operation) => + Expression.Lambda>(FuzzEngine.Splice(operation, row0, bound.Body), row0); // (bound ?? fallback) op v, with both values captured, so EF Core sends them as parameters. Expression> Coalesced(ExpressionType comparison, Expression> bound, T fallback) => Expression.Lambda>( Expression.MakeBinary(comparison, Expression.Coalesce(bound.Body, Captured(fallback)), Captured(v)), row0); - (string Name, Expression> Filter)[] filters = - [ - ("LowerBound() == null", On((_, x) => x.LowerBound() == null)), - ("LowerBound() != null", On((_, x) => x.LowerBound() != null)), - ("LowerBound().HasValue", On((_, x) => x.LowerBound().HasValue)), - ("UpperBound() == null", On((_, x) => x.UpperBound() == null)), - ("UpperBound() != null", On((_, x) => x.UpperBound() != null)), - ("(UpperBound() ?? max) >= v", Coalesced(ExpressionType.GreaterThanOrEqual, On((_, x) => x.UpperBound()), max)), - ("(LowerBound() ?? min) <= v", Coalesced(ExpressionType.LessThanOrEqual, On((_, x) => x.LowerBound()), min)) - ]; - - await using var context = new IntegrationDbContext(); - var mine = context.Reservations.Where(r => r.Id >= firstId && r.Id <= lastId); - var loaded = await mine.AsNoTracking().OrderBy(r => r.Id).ToListAsync(); - var failures = new List(); - - foreach (var (name, filter) in filters) - { - var memory = loaded.Where(filter.Compile()).Select(r => r.Id).ToList(); - await Agree(name, () => mine.Where(filter).Select(r => r.Id).OrderBy(id => id).ToListAsync(), memory); - } - - var projection = Expression.Lambda>(Expression.Coalesce(On((_, x) => x.UpperBound()).Body, Captured(max)), row0); - var projected = loaded.Select(projection.Compile()).ToList(); - try - { - var server = await mine.OrderBy(r => r.Id).Select(projection).ToListAsync(); - if (!server.SequenceEqual(projected)) - failures.Add($"Select(UpperBound() ?? max) on {label}: server [{string.Join(", ", server)}], memory [{string.Join(", ", projected)}]"); - } - catch (Exception e) when (e is InvalidOperationException or DbException) + async Task Sweep(string label, Expression> lower, Expression> upper) { - failures.Add($"Select(UpperBound() ?? max) on {label}: server threw {e.GetType().Name}: {e.Message}"); - } + (string Name, Expression> Filter)[] filters = + [ + ("LowerBound() == null", Over(lower, (_, x) => x == null)), + ("LowerBound() != null", Over(lower, (_, x) => x != null)), + ("LowerBound().HasValue", Over(lower, (_, x) => x.HasValue)), + ("UpperBound() == null", Over(upper, (_, x) => x == null)), + ("UpperBound() != null", Over(upper, (_, x) => x != null)), + ("(UpperBound() ?? max) >= v", Coalesced(ExpressionType.GreaterThanOrEqual, upper, max)), + ("(LowerBound() ?? min) <= v", Coalesced(ExpressionType.LessThanOrEqual, lower, min)) + ]; + + foreach (var (name, filter) in filters) + { + var memory = loaded.Where(filter.Compile()).Select(r => r.Id).ToList(); + await Agree(label, name, () => mine.Where(filter).Select(r => r.Id).OrderBy(id => id).ToListAsync(), memory); + } - // The filter a grid's "blank" header filter sends: the bound projected first, compared after. - var withUpper = On((r, x) => new { r.Id, To = x.UpperBound() }); - await Agree("Select(new { Id, To = UpperBound() }).Where(To == null)", - () => mine.Select(withUpper).Where(t => t.To == null).Select(t => t.Id).OrderBy(id => id).ToListAsync(), - [.. loaded.Select(withUpper.Compile()).Where(t => t.To == null).Select(t => t.Id)]); + var projection = Expression.Lambda>(Expression.Coalesce(upper.Body, Captured(max)), row0); + var projected = loaded.Select(projection.Compile()).ToList(); + try + { + var server = await mine.OrderBy(r => r.Id).Select(projection).ToListAsync(); + if (!server.SequenceEqual(projected)) + failures.Add($"Select(UpperBound() ?? max) on {label}: server [{string.Join(", ", server)}], memory [{string.Join(", ", projected)}]"); + } + catch (Exception e) when (e is InvalidOperationException or DbException) + { + failures.Add($"Select(UpperBound() ?? max) on {label}: server threw {e.GetType().Name}: {e.Message}"); + } - return failures; + // The filter a grid's "blank" header filter sends: the bound projected first, compared after. + var withUpper = Over(upper, (r, x) => new { r.Id, To = x }); + await Agree(label, "Select(new { Id, To = UpperBound() }).Where(To == null)", + () => mine.Select(withUpper).Where(t => t.To == null).Select(t => t.Id).OrderBy(id => id).ToListAsync(), + [.. loaded.Select(withUpper.Compile()).Where(t => t.To == null).Select(t => t.Id)]); + } - async Task Agree(string operation, Func>> server, List memory) + async Task Agree(string label, string operation, Func>> server, List memory) { try { diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/QueryTranslationTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/QueryTranslationTests.cs index 7a50a9e3..13231407 100644 --- a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/QueryTranslationTests.cs +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/QueryTranslationTests.cs @@ -262,6 +262,28 @@ public void LowerBound_InOrderBy() StringAssert.Contains(sql, "ORDER BY lower(b.\"Period\")"); } + /// + /// lower and upper are NULL for an empty or unbounded operand that is not NULL itself, + /// so a required column's bound can still be NULL. Declared as propagating its argument's + /// nullability, EF Core folded these to WHERE FALSE and dropped the COALESCE (#349). + /// ExecutedQueryTests.BoundsComparedWithNull_SelectTheRowsMemorySelects is the authority. + /// + [TestMethod] + public void BoundComparedWithNull_OnARequiredColumn_TestsTheBound() + { + var sql = Sql(db => db.Bookings.Where(b => b.Period.LowerBound() == null || b.BlockedDays.UpperBound() == null)); + StringAssert.Contains(sql, "lower(b.\"Period\") IS NULL"); + StringAssert.Contains(sql, "upper(b.\"BlockedDays\") IS NULL"); + Assert.IsFalse(sql.Contains("FALSE"), sql); + } + + [TestMethod] + public void BoundCoalesced_OnARequiredColumn_KeepsTheCoalesce() + { + var sql = Sql(db => db.Bookings.Where(b => (b.Period.UpperBound() ?? DateOnly.MaxValue) >= Day)); + StringAssert.Contains(sql, "COALESCE(upper(b.\"Period\") - 1, "); + } + [TestMethod] public void UpperBound_Discrete_CompensatesHalfOpenCanonicalization() { diff --git a/test/oracles/candidates.json b/test/oracles/candidates.json index 57ea2d31..d8638340 100644 --- a/test/oracles/candidates.json +++ b/test/oracles/candidates.json @@ -27,69 +27,6 @@ "issue": "#132", "method": "EveryEntryPoint_ReadsTheLiteralAsPostgresReadsIt" }, - { - "oracle": "ExecutedQueryTests", - "row": "LowerBound() == null on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "LowerBound() != null on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "LowerBound().HasValue on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "UpperBound() == null on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "UpperBound() != null on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "(UpperBound() ?? max) >= v on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "(LowerBound() ?? min) <= v on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "Select(UpperBound() ?? max) on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, - { - "oracle": "ExecutedQueryTests", - "row": "Select(new { Id, To = UpperBound() }).Where(To == null) on ", - "family": "lower()/upper() declared NULL only for a NULL argument", - "issue": "#349", - "method": "BoundsComparedWithNull_SelectTheRowsMemorySelects" - }, { "oracle": "ExecutedQueryTests", "row": "!IsEmpty() on DateRange? null:", diff --git a/test/oracles/claims.json b/test/oracles/claims.json index ba787ff9..506a2f3e 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -1210,12 +1210,14 @@ { "id": "E70-bounds-null-for-unbounded-or-empty-in-sql", "source": "docs/efcore.md#read-bounds-merge-ranges-and-aggregate", - "quote": "**`LowerBound()` and `UpperBound()` return `T?`.** PostgreSQL's `lower` and `upper` return `NULL` for an unbounded or empty range, and the in-memory methods match that.", - "claim": "In a query over a range column of any family, LowerBound() and UpperBound() are NULL exactly for the empty range and for the unbounded side, as in memory: compared with null, read through HasValue, coalesced with ?? in a filter or a projection, or compared with null after being projected, they select and return what the same expression gives over the rows as loaded.", + "quote": "**`LowerBound()` and `UpperBound()` return `T?`.** PostgreSQL's `lower` and `upper` return `NULL` for an unbounded or empty range, and the in-memory methods match that. So `== null`, `HasValue` and `??` on a bound select the same rows in a query as in memory, on a range or a `RangeSet` column.", + "claim": "In a query over a range or multirange column of any family, LowerBound() and UpperBound() are NULL exactly for the empty range or set and for the unbounded side, as in memory: compared with null, read through HasValue, coalesced with ?? in a filter or a projection, or compared with null after being projected, they select and return what the same expression gives over the rows as loaded.", "oracle": "differential", "surfaces": [ "RangeExtensions.LowerBound", - "RangeExtensions.UpperBound" + "RangeExtensions.UpperBound", + "RangeSet.LowerBound", + "RangeSet.UpperBound" ], "heldBy": [ "ExecutedQueryTests"