Skip to content

fix: declare lower() and upper() NULL for a non-null range - #353

Merged
CaffeinatedCoder merged 2 commits into
mainfrom
fix/349-bound-null-propagation
Oct 24, 2026
Merged

CaffeinatedCoder merged 2 commits into
mainfrom
fix/349-bound-null-propagation

Conversation

@CaffeinatedCoder

Copy link
Copy Markdown
Owner

Closes #349

Stacked on #352, which registers the disagreement as candidates. This PR removes the nine #349 entries, so merge #352 first; GitHub then retargets this PR to main.

Problem

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, as the in-memory LowerBound() and UpperBound() return null. So over a required column EF Core folded LowerBound() == null to WHERE FALSE, turned != null and HasValue into no filter, dropped the COALESCE of ?? fallback (open-ended rows fell out of a filter, and a projection threw Nullable object must have a value), and on a nullable column tested the bound with the column's own IS NULL. Every range type and RangeSet went through the same helper.

Change

  • ValueRangesMethodCallTranslator.BoundFunction: argumentsPropagateNullability: [false], keeping nullable: true. That covers lower and upper on ranges and on RangeSet, and the discrete upper(x) - 1, which EF Core still reduces to upper(x) IS NULL when it is compared with null.
  • ExecutedQueryTests.BoundsComparedWithNull_SelectTheRowsMemorySelects: each family now seeds every shape into its range column and, as a one-element multirange, into its RangeSet column (Seats/SeatBlocks, …, BillingPeriod/BillingPeriods), and runs all nine operations on both. The operations are written once over the bound and spliced onto x.LowerBound() and x.UpperBound() of each column.
  • QueryTranslationTests: two tests pin the SQL without a database: lower(b."Period") IS NULL and upper(b."BlockedDays") IS NULL with no FALSE, and COALESCE(upper(b."Period") - 1, …).
  • test/oracles/candidates.json: the nine LowerBound() and UpperBound() in a query: == null becomes WHERE FALSE and ?? max loses its COALESCE, because lower() and upper() are declared NULL only for a NULL argument #349 entries removed, 16 left. A null range answers some extensions in memory and throws on others, and the answers it gives disagree with PostgreSQL on a NULL column #350's twelve stay.
  • test/oracles/claims.json: E70 now names RangeSet.LowerBound/UpperBound, says "range or multirange column", and quotes the new docs/efcore.md sentence below.
  • docs/efcore.md: "So == null, HasValue and ?? on a bound select the same rows in a query as in memory, on a range or a RangeSet column."
  • docs/migration.md: a new ## Migration from v10.x section with ### Check bound comparisons with null in queries (what changed per query form, and how to find the calls).
  • Changelogs (root, EF Core, EF Core NodaTime): a ### Breaking line under [11.0.0-preview.1]. A silent fix into a major's preview goes under Breaking (ChangedAnswers_AppearOnlyInAMinor); the 10.x port gets the ### Changed answers row of its minor.

Tests

All local, SDK 11.0.100-rc.2.26504.105, Docker via OrbStack.

  • With the fix, before removing the candidates: the bound sweep fails with exactly the nine candidate fixed: remove the entry … lines and no other row, the new RangeSet rows included.
  • With the candidates removed: both sweeps pass (PredicatesOverANullableColumn still skips A null range answers some extensions in memory and throws on others, and the answers it gives disagree with PostgreSQL on a NULL column #350's twelve).
  • Fix reverted ([true] back): the bound sweep fails on 234 rows, 117 on the range columns and 117 on the RangeSet columns, none of them a known candidate; for example LowerBound() == null on SeatBlocks (RangeSet<Int32Range>): server rows {}, memory rows {(,a] (,b] (,) empty}. Both new translation tests fail too. Restored afterwards.
  • Full suite on postgres:16-alpine: all eight test projects pass (core 1397, NodaTime 238, EF 293, EF NodaTime 83, ASP.NET Core 92, conventions 407, integration 286, infinity-disabled 4).
  • ExecutedQueryTests on postgres:18-alpine: 15/15.

Not covered here: the release/10.x port, which needs a 10.x minor with a ### Changed answers row; its own issue is filed separately.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@CaffeinatedCoder CaffeinatedCoder added this to the 11.0.0 milestone Oct 24, 2026
@CaffeinatedCoder CaffeinatedCoder added bug Something isn't working silent Answers wrongly without throwing — fixes go in the next major, never deferred breaking Changes an answer or a signature — needs a major area/ranges Range unions, variants and the shape engines area/range-set RangeSet and multirange algebra package/efcore Affects CodoMetis.ValueRanges.EFCore.PostgreSQL package/efcore-nodatime Affects CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime severity/high Wrong results, crash or failed writes in a realistic scenario stacked Base is another open PR; merge in order, CI does not run until it targets main labels Oct 24, 2026
Base automatically changed from test/349-350-bound-null-candidates to main October 24, 2026 11:34
@CaffeinatedCoder CaffeinatedCoder removed the stacked Base is another open PR; merge in order, CI does not run until it targets main label Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/range-set RangeSet and multirange algebra area/ranges Range unions, variants and the shape engines breaking Changes an answer or a signature — needs a major bug Something isn't working package/efcore Affects CodoMetis.ValueRanges.EFCore.PostgreSQL package/efcore-nodatime Affects CodoMetis.ValueRanges.EFCore.PostgreSQL.NodaTime severity/high Wrong results, crash or failed writes in a realistic scenario silent Answers wrongly without throwing — fixes go in the next major, never deferred

Projects

None yet

1 participant