From 42040e6ad732ec551bcce6cefd499938f2bdc852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Sat, 24 Oct 2026 22:29:42 +0200 Subject: [PATCH] fix: Contains(T) over a NULL element is false in a query (#381) Rule (c) guarded a NULL range, multirange or value set operand, but not the element. Over the optional side of an outer join, or a nullable column read with .Value, r."Period" @> e was NULL: the negation dropped the row EF Core keeps for its own comparisons, a projection into bool threw, and All over a captured list selected the NULL rows. The predicate wrapper now checks the element too (e IS NOT NULL AND ...); EF Core drops the check for a required element. Found-By: OT Defect-Class: SQL Co-Authored-By: Claude Opus 5.5 (cherry picked from commit c976194c80e9fbdcc92bc980147f6c777051000f) --- CHANGELOG.md | 1 + docs/efcore.md | 4 +- docs/migration.md | 7 +- .../CHANGELOG.md | 1 + .../CHANGELOG.md | 1 + .../Query/ValueRangesMethodCallTranslator.cs | 32 ++++-- .../NullElementTests.cs | 103 ++++++++++++++++++ .../NullOperandTranslationTests.cs | 22 ++++ test/oracles/claims.json | 5 +- 9 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/NullElementTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 14d53f86..c9729ca7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Some calls answer differently: [Changed answers in 10.2.0](https://github.com/Ca - Ranges: a predicate called on a `null` range, or given a `null` range or set, returns `false`, as a query does over a NULL column. `Contains(Empty)`, `IsContainedBy((,))` and `(,).Contains(null)` returned `true`, and `Contains(T)` and a `null` set operand threw ([#357](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/357), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). - 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 ([#354](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/354), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). - EF Core: a range, multirange or value set predicate over a NULL column is `false`, so its negation keeps the row, as EF Core does for `string` and `string[]`. `!b.Period.Overlaps(p)` dropped the NULL rows, and a projected predicate threw ([#357](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/357), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). +- EF Core: `Contains(T)` on a range or multirange is `false` in a query when the element is NULL: a column of a missing child in an outer join, or a nullable column read with `.Value`. Its negation dropped the row, and a projection threw ([#387](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/387), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). ### Fixed diff --git a/docs/efcore.md b/docs/efcore.md index 046febfa..a0697107 100644 --- a/docs/efcore.md +++ b/docs/efcore.md @@ -91,7 +91,9 @@ EF Core does for a `string` or `string[]` column, and what the methods return in range. So `!b.Period.Overlaps(other)` keeps the rows whose `Period` is NULL, as `!b.Name.StartsWith("a")` keeps the rows whose `Name` is NULL. The SQL adds `b."Period" IS NULL OR NOT (…)` for a nullable column and nothing for a required one. Value set -predicates follow the same rule in a query, though in memory they throw for a `null` set. +predicates follow the same rule in a query, though in memory they throw for a `null` set. So does +`Contains(day)` when `day` is NULL, as it is for a column of a missing child in an outer join or a +nullable column read with `.Value`. `CompareTo` keeps its own rule for a NULL column ([Sort ranges](ranges.md#sort-ranges)). ### Read bounds, merge ranges and aggregate diff --git a/docs/migration.md b/docs/migration.md index 30903f52..8cee0e3a 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -901,9 +901,12 @@ whether the predicate was negated or not, and a projected predicate threw when i | `Where(b => b.Period!.Overlaps(p))` | no NULL rows | no NULL rows | | `Select(b => b.Period!.IsEmpty())` | throws (`InvalidOperationException`) at a NULL row | `false` for it | | `Where(b => !b.Tags!.Contains("x"))` on a nullable value set | the NULL rows are missing | the NULL rows are included | +| `Where(b => !b.Period.Contains(b.Visit!.On))`, for a booking without a visit | the row is missing | the row is included | +| `Select(b => b.Period.Contains(b.Day!.Value))` on a nullable `DateOnly? Day` | throws (`InvalidOperationException`) at a NULL `Day` | `false` for it | -The SQL of a required column doesn't change. A nullable one gets `b."Period" IS NULL OR NOT (…)` -under a negation. +The same goes for the element `Contains(T)` looks for, when it is NULL: a required column of a +missing child in an outer join, or a nullable column read with `.Value`. The SQL of a required +column doesn't change. A nullable one gets `b."Period" IS NULL OR NOT (…)` under a negation. - **Search for negated predicates over a nullable column.** A `!` that silenced the nullable warning, as in `!b.Period!.Overlaps(p)`, often marks the place. diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md index 3fcd3abc..8c8a72f6 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime/CHANGELOG.md @@ -9,6 +9,7 @@ covers all four packages, which share one version number and release together. - 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 ([#354](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/354), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). - EF Core: a range, multirange or value set predicate over a NULL column is `false`, so its negation keeps the row, as EF Core does for `string` and `string[]`. `!b.Period.Overlaps(p)` dropped the NULL rows, and a projected predicate threw ([#357](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/357), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). +- EF Core: `Contains(T)` on a range or multirange is `false` in a query when the element is NULL: a column of a missing child in an outer join, or a nullable column read with `.Value`. Its negation dropped the row, and a projection threw ([#387](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/387), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). ## [10.1.0] — 2026-10-19 diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md index d9cfce4d..73c2223d 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md @@ -9,6 +9,7 @@ covers all four packages, which share one version number and release together. - 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 ([#354](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/354), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). - EF Core: a range, multirange or value set predicate over a NULL column is `false`, so its negation keeps the row, as EF Core does for `string` and `string[]`. `!b.Period.Overlaps(p)` dropped the NULL rows, and a projected predicate threw ([#357](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/357), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). +- EF Core: `Contains(T)` on a range or multirange is `false` in a query when the element is NULL: a column of a missing child in an outer join, or a nullable column read with `.Value`. Its negation dropped the row, and a projection threw ([#387](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/387), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#changed-answers-in-1020)). ### Fixed diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs index d2d2da27..a3bf2905 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesMethodCallTranslator.cs @@ -113,9 +113,11 @@ private static bool TakesSet(MethodInfo method) /// /// A predicate with a NULL range or multirange on either side is false: what EF Core answers /// for a predicate over NULL on its own types, and what the in-memory predicates answer for a null - /// (#350, #355). Each range or multirange operand becomes x IS NOT NULL AND <predicate>. - /// EF Core's nullability processing drops the check for a required column and a non-null parameter, - /// and writes x IS NULL OR NOT (…) under a negation, so a negated predicate keeps the NULL row. + /// (#350, #355), and so is one with a NULL element: Contains(T) over the optional side of an + /// outer join or a nullable column read with .Value (#381). Each range, multirange or element + /// operand becomes x IS NOT NULL AND <predicate>. EF Core's nullability processing drops + /// the check for a required column and a non-null parameter, and writes x IS NULL OR NOT (…) + /// under a negation, so a negated predicate keeps the NULL row. /// /// /// CompareTo never reaches this: EF Core turns a.CompareTo(b) op k into a comparison, @@ -123,15 +125,27 @@ private static bool TakesSet(MethodInfo method) /// private SqlExpression FalseOverNull(SqlExpression predicate, IReadOnlyList operands) { - for (var index = operands.Count - 1; index >= 0; index--) + var unwrapped = operands.Select(Unwrap).ToList(); + + // The element type is the one of the range or multirange the predicate is over. + IRangeTypeDefinition? ranges = null; + foreach (var operand in unwrapped) + if (RangeTypeRegistry.TryGetByClrType(operand.Type, out ranges, out _)) break; + + for (var index = unwrapped.Count - 1; index >= 0; index--) { - var operand = Unwrap(operands[index]); - if (!RangeTypeRegistry.TryGetByClrType(operand.Type, out var definition, out var isSet)) + var operand = unwrapped[index]; + + SqlExpression mapped; + if (RangeTypeRegistry.TryGetByClrType(operand.Type, out var definition, out var isSet)) + mapped = operand.TypeMapping is null + ? sqlExpressionFactory.ApplyTypeMapping(operand, isSet ? definition.RangeSetTypeMapping : definition.RangeTypeMapping) + : operand; + else if (ranges is not null && (Nullable.GetUnderlyingType(operand.Type) ?? operand.Type) == ranges.ElementClrType) + mapped = operand.TypeMapping is null ? ApplyElementMapping(operand, ranges) : operand; + else continue; - var mapped = operand.TypeMapping is null - ? sqlExpressionFactory.ApplyTypeMapping(operand, isSet ? definition.RangeSetTypeMapping : definition.RangeTypeMapping) - : operand; predicate = sqlExpressionFactory.AndAlso(sqlExpressionFactory.IsNotNull(mapped), predicate); } diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/NullElementTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/NullElementTests.cs new file mode 100644 index 00000000..36210e2a --- /dev/null +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/NullElementTests.cs @@ -0,0 +1,103 @@ +using Microsoft.EntityFrameworkCore; +using Npgsql; + +namespace CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests; + +/// +/// A NULL element against live PostgreSQL: a nullable column read with .Value, which is what the +/// optional side of an outer join gives too. Row 1 has a Day inside every range, row 2 has none. +/// In memory the call cannot be made (.Value throws), so the reference is rule (c), EF Core's own +/// convention: a predicate over NULL is false and its negation true (#350, #381). +/// +/// +/// On main the composition sweeps (OuterJoinProbeTests, NullableOperandPairProbeTests, +/// QuantifierProbeTests) hold this over every shape; this line has none of them, so the ports +/// (#387) hold the cases the issues name here. +/// +[TestClass] +public class NullElementTests +{ + private static DateOnly March(int day) => new(2026, 3, day); + + public sealed class Row + { + public int Id { get; set; } + + public DateRange Period { get; set; } = DateRange.Empty; + + public RangeSet Blocks { get; set; } = RangeSet.Empty; + + public DateOnly Start { get; set; } + + public DateOnly? Day { get; set; } + } + + /// A database of its own, so that the nullable element column does not change the shared schema. + private sealed class Context(string connectionString) : DbContext + { + public DbSet Rows => Set(); + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString, npgsql => npgsql.UseValueRanges()); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + => modelBuilder.Entity().Property(r => r.Id).ValueGeneratedNever(); + } + + private static async Task Seeded() + { + ContainerLifecycle.RequireServer(); + + var name = $"null_element_{Guid.NewGuid():N}"; + await using (var connection = new NpgsqlConnection(ContainerLifecycle.ServerConnectionString)) + { + await connection.OpenAsync(); + await using var command = new NpgsqlCommand($"CREATE DATABASE {name}", connection); + await command.ExecuteNonQueryAsync(); + } + + var context = new Context(new NpgsqlConnectionStringBuilder(ContainerLifecycle.ServerConnectionString) { Database = name }.ConnectionString); + await context.Database.EnsureCreatedAsync(); + + var period = DateRange.CreateFinite(March(1), March(10), true, true); + context.Rows.AddRange( + new Row { Id = 1, Period = period, Blocks = RangeSet.From([period]), Start = March(3), Day = March(7) }, + new Row { Id = 2, Period = period, Blocks = RangeSet.From([period]), Start = March(3), Day = null }); + await context.SaveChangesAsync(); + context.ChangeTracker.Clear(); + return context; + } + + private static async Task Ids(IQueryable rows) => [.. await rows.Select(r => r.Id).OrderBy(id => id).ToListAsync()]; + + [TestMethod] + public async Task Contains_OverANullElement_IsFalse_AndItsNegationTrue() + { + await using var db = await Seeded(); + + CollectionAssert.AreEqual(new[] { 1 }, await Ids(db.Rows.Where(r => r.Period.Contains(r.Day!.Value)))); + CollectionAssert.AreEqual(new[] { 2 }, await Ids(db.Rows.Where(r => !r.Period.Contains(r.Day!.Value)))); + CollectionAssert.AreEqual(new[] { 1 }, await Ids(db.Rows.Where(r => r.Blocks.Contains(r.Day!.Value)))); + CollectionAssert.AreEqual(new[] { 2 }, await Ids(db.Rows.Where(r => !r.Blocks.Contains(r.Day!.Value)))); + } + + [TestMethod] + public async Task Contains_OverANullElement_ProjectsFalse() + { + await using var db = await Seeded(); + + // It threw InvalidOperationException reading the NULL into bool. + CollectionAssert.AreEqual(new[] { true, false }, await db.Rows.OrderBy(r => r.Id).Select(r => r.Period.Contains(r.Day!.Value)).ToListAsync()); + } + + [TestMethod] + public async Task All_OverACapturedList_DropsTheNullElement() + { + await using var db = await Seeded(); + var windows = new List { DateRange.CreateFinite(March(3), March(8), true, true) }; + + // EF Core writes All as NOT EXISTS (… WHERE NOT (x @> e)); a NULL inner NOT made it true. + CollectionAssert.AreEqual(new[] { 1 }, await Ids(db.Rows.Where(r => windows.All(x => x.Contains(r.Day!.Value))))); + CollectionAssert.AreEqual(new[] { 2 }, await Ids(db.Rows.Where(r => !windows.All(x => x.Contains(r.Day!.Value))))); + } +} diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/NullOperandTranslationTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/NullOperandTranslationTests.cs index 0fb20e25..625214b1 100644 --- a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/NullOperandTranslationTests.cs +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/NullOperandTranslationTests.cs @@ -24,6 +24,10 @@ public sealed class Row public RangeSet? MaybeBlocks { get; set; } public StringSet? MaybeTags { get; set; } + + public DateOnly Start { get; set; } + + public DateOnly? Day { get; set; } } private sealed class Context : DbContext @@ -82,4 +86,22 @@ public void AProjectedPredicate_IsFalseForANullColumn() var sql = context.Rows.Select(r => r.Maybe!.IsEmpty()).ToQueryString(); StringAssert.Contains(sql, "SELECT r.\"Maybe\" IS NOT NULL AND isempty(r.\"Maybe\")"); } + + /// + /// An element is NULL on the optional side of an outer join or in a nullable column read with + /// .Value, and Contains(T) over it is false too (#381): NOT (r."Period" @> + /// r."Day") dropped the row EF Core keeps for its own comparisons, and a projection threw. + /// + [TestMethod] + public void ANullableElement_IsCheckedForNull() + { + Assert.AreEqual("r.\"Day\" IS NOT NULL AND r.\"Period\" @> r.\"Day\"", Where(rows => rows.Where(r => r.Period.Contains(r.Day!.Value)))); + Assert.AreEqual("r.\"Day\" IS NULL OR NOT (r.\"Period\" @> r.\"Day\")", Where(rows => rows.Where(r => !r.Period.Contains(r.Day!.Value)))); + Assert.AreEqual( + "r.\"MaybeBlocks\" IS NULL OR r.\"Day\" IS NULL OR NOT (r.\"MaybeBlocks\" @> r.\"Day\")", + Where(rows => rows.Where(r => !r.MaybeBlocks!.Contains(r.Day!.Value)))); + + // A required element keeps its SQL. + Assert.AreEqual("NOT (r.\"Period\" @> r.\"Start\")", Where(rows => rows.Where(r => !r.Period.Contains(r.Start)))); + } } diff --git a/test/oracles/claims.json b/test/oracles/claims.json index 1af7d9e4..6f3cbcb7 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -1141,7 +1141,7 @@ "id": "E71-predicate-over-a-null-column-is-false", "source": "docs/efcore.md#query-ranges-with-linq", "quote": "On a nullable column, a predicate over a NULL value is `false`, and its negation `true`.", - "claim": "Over a nullable range, multirange and value set column, every discovered Boolean predicate of RangeExtensions, RangeSet and ValueSetExtensions, with the column on either side, selects in a filter, a negated filter and a projection what the same expression selects over the rows as loaded, with false for a NULL row where memory throws; a required column's SQL carries no null check, and a nullable one reads x IS NULL OR NOT (…) under a negation.", + "claim": "Over a nullable range, multirange and value set column, every discovered Boolean predicate of RangeExtensions, RangeSet and ValueSetExtensions, with the column on either side, selects in a filter, a negated filter and a projection what the same expression selects over the rows as loaded, with false for a NULL row where memory throws, and the same for Contains(T) over a NULL element (the optional side of an outer join, or a nullable column read with .Value); a required column's SQL carries no null check, and a nullable one reads x IS NULL OR NOT (…) under a negation.", "oracle": "differential", "surfaces": [ "RangeExtensions.Overlaps", @@ -1151,7 +1151,8 @@ ], "heldBy": [ "ExecutedQueryTests", - "NullOperandTranslationTests" + "NullOperandTranslationTests", + "NullElementTests" ] }, {