Repository navigation
fix: strict temporal parsing, temporal store text as Npgsql binds it, ExecuteUpdate guard (9.0.0) - #32
Merged
Conversation
The BCL temporal ranges and closed sets parsed element text with the
lenient BCL Parse, which reads the invariant culture month first:
DateRange.Parse("[01.02.2027,05.03.2027)") was January 2 to May 2, and
a DateTimeOffset without an offset took the parsing machine's. The
wrapper arities handed the text to their element's own, possibly
lenient, Parse. All of it was silent.
Element text now goes through IsoTemporal, which accepts exactly the
forms this library writes and PostgreSQL's ISO-DateStyle output (space
separator, +05 offsets, up to seven fraction digits), HH:mm without
seconds, and infinity / -infinity as MaxValue / MinValue - what Npgsql
materializes for them. Everything else is a FormatException through the
existing bounded-excerpt path. A DateTimeOffset requires its offset.
The wrapper arities, BCL and NodaTime, parse as the primitive first and
hand the element its own round-trip form, so a lenient element parser
never sees non-ISO text. NodaTime reads infinity for Instant, LocalDate
and LocalDateTime, the types Npgsql maps it for (measured).
TemporalLiteralGrammarTests pins the grammar; the conventions sweep
TemporalLiteralStrictnessContractTests refuses a non-ISO element through
Parse, TryParse, the span overload, JSON and the TypeConverter for all
26 temporal range and set types. Restoring a lenient ParseValue on
DateRange, DateTimeOffsetRange or DateSet<T> fails the sweep by name.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PostgreSQL writes a historical local-mean-time offset with seconds (Asia/Kolkata before 1854 is +05:53:28), for instance into a JSON document through to_jsonb. DateTimeOffset cannot hold such an offset, so the strict parser refused it and a row holding one would not load. It now applies the offset to the wall clock and keeps the instant at offset zero; offsets beyond +-14:00 or with 60 seconds stay refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A temporal value the EF provider writes as text - inside a JSON column
(new in 9.0.0), or as a SQL literal - carried seven fraction digits
(nine for NodaTime) and let PostgreSQL round them half-to-even, while
Npgsql truncates a parameter and a native column to the microsecond
toward 2000-01-01. For a remainder above 500 ns after 2000 (below it
before), the document and the constant held a different value from the
column: Doc.X == v missed its own row, Contains disagreed at the bounds,
and a range narrower than a microsecond was empty only on the server.
MinValue / MaxValue stayed finite in text while a parameter carried
-infinity / infinity. EF 10 inlines evaluatable expressions as
constants, so the literal path was reachable without EF.Constant.
The store text is now the value Npgsql would send:
- StoreTemporal / NodaStoreTemporal truncate as Npgsql does and spell
the extremes it maps (measured: DateTime, DateTimeOffset, DateOnly,
Instant, LocalDate, LocalDateTime) as infinity, honouring
Npgsql.DisableDateTimeInfinityConversions. Applied as a separate
storeValue after normalizeValue, which stays the core element
normalization.
- Ranges and multiranges are rebuilt through their factory after
truncation, so they collapse and merge as the server would.
- Value sets deduplicate the store values on every path; a native array
no longer holds two equal timestamps whose cardinality disagrees with
Count. Temporal sets write a JSON document as their store primitives.
- The JSON writer resolves the store text through the registries on
first use, so a compiled model that never built a wrapper family's
definition still writes the store form; an unregistered type throws.
- YearMonth types route their date form through the LocalDate store
text (9999-12 ends on infinity). YearMonthSet<T> in a JSON column is
written as dates - its own {2024-06} form could not be cast to date[],
so every query into it failed.
TemporalStoreParityTests (integration) builds every temporal type from
seven probes - remainders above, below and at 500 ns, before 2000, the
extremes, sub-microsecond ranges - and asserts, per property, that the
server reads the document as the column, that parameters and constants
match both copies, that the copies materialize equal, and that Contains
agrees with memory around every bound. Removing the BCL truncation, the
NodaTime truncation, the infinity text or the set dedupe fails it with
165, 127, 46 and 16 findings. TemporalStoreTextTests pins the text, and
a never-resolved wrapper set stands in for a compiled model.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EF Core translates such a setter to jsonb_set(doc, path, to_jsonb(@p)), binding the set's PostgreSQL array: the document then holds a JSON array where it holds the set's literal string everywhere else. The update reported success; afterwards the entity no longer loaded ("Cannot get the value of a token type 'StartArray' as a string") and every query on the property failed with a malformed array literal. The query interceptor now refuses it at compilation, before a connection opens, naming the property and pointing at SaveChanges or a whole-document SetProperty. It matches EF Core's private ExecuteUpdate lowering by name, declaring type and parameter shape rather than the internal ExecuteUpdateMethodInfo (EF1001), walks the setter's member chain through the model's complex properties and refuses only a set whose declaring type IsMappedToJson. Ranges and multiranges stay allowed: to_jsonb writes PostgreSQL's range output, which the strict parser reads. ExecuteUpdateGuardTests pins the refusal (top-level and nested) and the shape of EF's lowering, so an EF upgrade that changes it fails loudly. ExecuteUpdateJsonTests (integration) sets temporal and plain ranges and a multirange inside a document and reads them back, sets the whole document and a native set column, and checks the refused row still loads. Removing the guard fails both; removing the space-separated form from the strict parser fails the read-back with a FormatException. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…bility - Changelogs (root and the four packages): strict ISO 8601 temporal parsing joins "what can break you"; Fixed entries for temporal SQL constants at the database's resolution and set deduplication; the JSON-column entry states the store representation, the temporal rule and the ExecuteUpdate refusal. - docs/efcore.md: the document holds the store's text, not the API JSON (a table of where they differ), and is a persisted format; what works inside a document, measured against PostgreSQL 17. - docs/serialization.md: the temporal grammar beside the numeric one; a custom-format ToString is display text; DateTimeOffsetSet JSON keeps System.Text.Json's local-offset rule. - docs/migration.md v8.x: strict temporal literals with the call-site edits, and temporal values reaching the database at its resolution. - Extensibility: the range interfaces are the shipped types' shared vocabulary, not an extension point, and may gain members in a minor version (IRangeFactory remarks, docs/ranges.md, docs/architecture.md). - SetJsonConverterTests pins null versus empty for sets and multiranges. - CLAUDE.md: rules for strict temporal parsing, temporal store text and the ExecuteUpdate refusal; the wrapper-arity rule now names the core. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ution Found by the value-semantics review of this branch. InstantSet and LocalTimeSet (and the BCL TimeSet) have no element normalization, so a bare element probe kept the provider's element mapping. A parameter is fine there - Npgsql truncates on the wire - but a constant is not: Npgsql renders a NodaTime Instant or LocalTime constant with nine fraction digits, which the server rounds, so InstantSet.Contains(EF.Constant(x)) answered false for the element x was stored as, while in memory it answered true. The bridged element mapping is now built whenever the family has a store resolution, not only a normalization, so the constant carries the truncated value. TemporalStoreParityTests gains the check that an element probed as a constant matches exactly as it does as a parameter, on the column and in the document; keying the mapping on normalizeValue alone again fails it for every NodaTime instant and time set element. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
r.Period == DateRange.CreateFinite(new DateOnly(2026, 1, 1), ...) failed with "Expression '[2026-01-01,...]' in the SQL tree does not have a type mapping assigned". EF Core evaluates a call that depends on nothing in the query on the client and inlines the result as a constant typed by its value - the sealed variant, DateRange.Finite, under a conversion back to DateRange. No type mapping exists for a variant, and EF does not carry the column's mapping across to an operand of another CLR type, so ==, !=, Equals and a conditional failed for every range type, including Parse(...) and the unbounded factories. Contains and Overlaps were unaffected (their translators type the operand), and so were captured variables and EF.Constant. No translator can help: the call is gone before translation. The query interceptor now retypes such a constant to its range union, which is what EF.Constant ends up doing; the literal then goes through the store text like any other range constant. A factory call that depends on a column still reaches the existing range-constructor translation. InlineRangeConstantTests pins the SQL for the operators, every shape, Parse and a temporal range at the store's resolution; NodaInlineRangeConstantTests covers InstantRange and YearMonthRange's date form; InlineRangeConstantExecutionTests (integration) matches an inline constant against its own row for all 11 range types. Removing the retyper fails all three with the original error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A closed DateTimeOffsetSet read its JSON elements through System.Text.Json's own converter, which reads an ISO timestamp without an offset - or a bare date - in the parsing machine's offset: the same payload held different instants on different servers, while the set's literal refused the text. DateTimeSet took a bare date as midnight, and TimeSet an unpadded "9:30". When the element would be read by System.Text.Json's built-in converter for a BCL temporal type (compared by converter type, so a converter registered on the options or substituted by a source-generated context still reads), the set converter now reads the string through the family's strict ParseValue, with the usual bounded-excerpt JsonException. The write side is unchanged and reads back. The grammar widens where it was needlessly strict, so JSON clients keep working: any number of fraction digits, truncated to 100 ns as System.Text.Json truncates them (Go and Java write nine), and a +-HH offset for a DateTime. TemporalSetJsonTests pins the refusals, nine-digit input, the unchanged written form, a registered converter winning and a source-generated context; the strictness sweep now probes offset-less and date-only forms through every entry point; the NativeAOT consumer round-trips a DateTimeOffsetSet and refuses an offset-less element. Disabling the strict read fails the unit tests and the sweep; removing the fraction truncation fails the nine-digit cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A range's element operand - range.Contains(x), a constructor argument -
fell back to the provider's element mapping, which knows neither the
range's normalization nor the store's resolution:
- DateTimeOffsetRange.Contains(dto) threw for any non-UTC offset
("Cannot write DateTimeOffset with Offset=05:30:00 ... only offset 0
(UTC) is supported"), DateTimeRange.Contains likewise for a
UTC-kinded DateTime;
- an element constant was Npgsql's literal: nine fraction digits for
NodaTime, which the server rounds, and before 2000 a cut toward 0001
rather than toward the PostgreSQL epoch - so
range.Contains(EF.Constant(x)) could disagree with the same probe as
a parameter.
Every temporal RangeTypeDefinition now supplies RangeElementTypeMapping:
parameters go through the definition's normalization and store
resolution, and constants are the store text in PostgreSQL's typed-
literal form (TIMESTAMP '...', TIMESTAMPTZ '...', DATE '...',
TIME '...'), still self-describing as the polymorphic range operators
require. Ranges without a normalization keep the provider's mapping.
TemporalStoreParityTests probes each range with its original sub-
microsecond bounds as a parameter and as a constant; its +05:30
DateTimeOffset probes threw before. TemporalStoreTextTests pins the
literals. Removing the mapping fails both: 21 parity findings, the
offset throw among them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The element checks compared a parameter with a constant, so both missing passed. A set element the set was built from must now match its own row both ways; a range's Contains on its original sub-microsecond bounds must answer as the materialized range does for the probe as stored, computed in the test rather than by the plugin. Document copies are probed too. The matrix now holds every temporal wrapper family: each is a separate registration that has to pass its own truncation, and only DateTimeSet<T> was exercised below a microsecond. Seeded: a DateTimeOffset element operand shifted one microsecond early in both paths passes the old checks and fails these with 42 lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ExecuteUpdate was covered, the path an application takes was not. On the complex document (rewritten whole on every save) and on an owned one (jsonb_set for a lone changed property), a range and a value set are replaced and a nullable range, temporal range and value set go from null to a value and back, one change per save; untouched properties are checked after each phase, read back and queried on the server. Seeded: a value comparer that misses replacements fails both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
StoreTemporal honours Npgsql.DisableDateTimeInfinityConversions, but the switch is read once per process, so it cannot be tested inside the other integration run. A project of its own sets it from its runtimeconfig and checks MinValue / MaxValue stay finite and truncated in the document, the literal and the element operands, as Npgsql then sends them. Its first test checks the switch reached the process. Seeded: the plugin ignoring the switch fails every path; dropping the runtimeconfig option fails all three tests, the switch check by name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The integration job named one project. It now runs each test/*IntegrationTests/, which the unit job already skips. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The parity matrix covers each registered store conversion, not every closed type; the JSON writer writes the store's text, which is not ToString() for temporal values or the YearMonth types; the tracked-update tests check untouched properties per phase. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pre-tag defects from the release-readiness probes against a live PostgreSQL 17, the ones a value-semantics review of this branch found, and the three items brought forward from 9.0.1. Each returned a wrong value or threw on a valid query.
1. Strict ISO 8601 temporal parsing (breaking, so it's 9.0.0 or 10.0)
The BCL temporal types parsed element text with the lenient BCL
Parse, which reads month first:DateRange.Parse("[01.02.2027,05.03.2027)")was January 2 to May 2.DateTimeOffsetwithout an offset took the parsing machine's offset.All of it was silent, through
Parse, JSON, theTypeConverterand configuration binding.IsoTemporalnow accepts exactly what this library writes and what PostgreSQL prints in its ISODateStyle:yyyy-MM-ddHH:mmorHH:mm:ss, with any number of fraction digits, truncated to 100 nsTor a space between date and timeZ,±HH,±HH:mmor±HH:mm:ssoffsets (required for aDateTimeOffset)infinity/-infinity, read as the extreme values, which is what Npgsql materializes for themThe wrapper arities, BCL and NodaTime, parse the text as their primitive first. NodaTime was already strict and now also reads
infinity.Closed temporal sets read their JSON elements with the same grammar. System.Text.Json's own converter read an offset-less
DateTimeOffset, or a bare date, in the machine's offset. A converter you register for the element type still wins, and the written form is unchanged.2. Temporal values written as text are the value Npgsql binds
The JSON writer (new in 9.0.0) and the SQL literals sent 7 or 9 fraction digits, which PostgreSQL rounds half-to-even. Npgsql instead truncates parameters and native columns toward 2000-01-01, so a value could miss its own row, and
MinValue/MaxValuestayed finite in text while a parameter carried±infinity.What changed:
StoreTemporalandNodaStoreTemporaltruncate as Npgsql does and spell the extremes asinfinity. They're applied as a separatestoreValue.YearMonthtypes: a range ending in 9999-12 ends oninfinity, andYearMonthSet<T>in JSON is written as dates.3. Element operands get the range's or set's own mapping
Sets:
InstantSet/LocalTimeSet.Contains(EF.Constant(x))used Npgsql's nine-digit literal and missed the element it was stored from. This one was found by the review.Ranges:
DateTimeOffsetRange.Contains(dto)threw for any non-UTC offset, andDateTimeRange.Containsfor a UTC-kindedDateTime.Every temporal range now has
RangeElementTypeMapping: parameters are normalized and truncated, and constants are the stored value in PostgreSQL's typed-literal form.4. A range built inline in a query translates
r.Period == DateRange.CreateFinite(new DateOnly(2026, 1, 1), …),Int32Range.Parse("[1,5]", null)and similar threw "does not have a type mapping assigned" under==,!=,Equalsand inside conditionals. EF Core evaluates such a call on the client and inlines a constant typed as the sealed variant. A translator can't help, because the call is gone before translation. The query interceptor now retypes the constant to its range type.5.
ExecuteUpdateof a value set inside a JSON column is refusedEF Core writes such a setter with
to_jsonb, which stores a JSON array where the document holds the set's literal; the row then no longer loads. The guard matches EF's private lowering by shape, and a test pins that shape. Ranges, multiranges and the whole JSON property can still be set.Docs and policy
docs/efcore.md: the document holds the store's text, not the API JSON; it's a persisted format; and a support matrix measured against the server.Review follow-ups (tests and CI only)
Containson its original sub-microsecond bounds must answer as the materialized range does for the probe as stored. The matrix now covers every temporal wrapper family, because each one is a separate registration with its own truncation.jsonb_setfor a lone change. Values are changed one per save; untouched properties are checked after each phase.Npgsql.DisableDateTimeInfinityConversionsgets its own test project, because the switch is read once per process. It shows the extremes stay finite and truncated in the document, the literal and the element operands. CI's integration job now runs everytest/*IntegrationTests/.Verification
Every fix was checked by removing it and confirming its test fails for the expected reason:
ParseValuerestoredTemporalStoreParityTests: 165 / 127 / 46 / 16 findingsYearMonthfamily store textExecuteUpdateguardExecuteUpdateread-backDateTimeOffsetelement operand shifted 1 µs on both pathsTest plan
Release gate run on a fresh clone of
e1a7671, inrelease.ymlorder:dotnet build -c Release: 0 warningsCI=true dotnet test -c Release --no-build: 1,481 passing, including 95 + 3 against a live PostgreSQL (both integration projects)dotnet pack -c Release --no-build -o dist: package validation against 8.0.0 clean./test/consumer-smoke-test.sh dist: passes, including the NativeAOT consumer with no trim or AOT warnings🤖 Generated with Claude Code