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"