fix(postgresql): type a DateTime parameter from its Kind, not its CLR type (weasel#403) - #409
Merged
Merged
Conversation
… type (weasel#403)
AddNamedParameter with a UTC DateTime threw at execution time:
ArgumentException: Cannot write DateTime with Kind=UTC to PostgreSQL type
'timestamp without time zone', consider using 'timestamp with time zone'
When no explicit type is supplied, Weasel typed the parameter from the
value's CLR type, and PostgresqlProvider maps typeof(DateTime) to Timestamp
unconditionally. But Postgres' choice depends on the *value*: Npgsql
resolves Kind=Utc to timestamptz and Kind=Local/Unspecified to timestamp. A
per-type mapping cannot express that, so every UTC DateTime got the wrong
type and was rejected on write. DateTime[] and List<DateTime> failed the
same way via the array branch.
Add IDatabaseProvider<,,>.ToParameterTypeForValue(object), defaulting to
today's ToParameterType(value.GetType()) so no other provider changes
behaviour. It is a default interface method plus a public virtual on
DatabaseProvider<,,>, so external implementers are unaffected.
PostgresqlProvider overrides it to read DateTime.Kind, for scalars and for
IReadOnlyList<DateTime> (covering arrays and List<T>; Npgsql forbids mixing
Kinds in one array, so the first element decides).
Three call sites were typing per-CLR-type and now route through it:
- DatabaseProvider.AddNamedParameter
- CommandBuilderBase.AppendParameter -- same bug, reached via the interface
- CommandExtensions.With(command, name, DateTime) -- hard-coded
NpgsqlDbType.Timestamp, so .With(name, DateTime.UtcNow) always threw
Column types are unaffected and stay per-type: a column has one type, only
parameters follow their value. GetDatabaseType(typeof(DateTime)) still
returns "timestamp without time zone", pinned by a test.
Verified against a live Postgres: UTC scalar, UTC array, UTC List and the
empty array now all round-trip, and string/Guid/int/DateTimeOffset/string[]
are unchanged. Postgres suite 796 passed, Core 21, SQLite 361.
Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Closes #403. Narrows #404.
The bug
Verified against a live Postgres. It only ever surfaced at execution time, so a unit assertion on the stamped type would not have caught the original report.
Cause
With no explicit type, Weasel typed the parameter from the value's CLR type, and
PostgresqlProvidermapstypeof(DateTime)toTimestampunconditionally. But Postgres' choice depends on the value — Npgsql resolvesKind=Utctotimestamptz,Kind=Local/Unspecifiedtotimestamp. A per-type mapping cannot express that.DateTime[]andList<DateTime>failed identically through the array branch.Change
New
IDatabaseProvider<,,>.ToParameterTypeForValue(object), defaulting to today'sToParameterType(value.GetType())— so no other provider changes behaviour. It is a default interface method plus apublic virtualonDatabaseProvider<,,>, so external implementers are unaffected.PostgresqlProvideroverrides it to readDateTime.Kind, for scalars and forIReadOnlyList<DateTime>(covers arrays andList<T>; Npgsql forbids mixing Kinds in one array, so the first element decides).Three call sites were typing per-CLR-type and now route through it:
DatabaseProvider.AddNamedParameterCommandBuilderBase.AppendParameterCommandExtensions.With(cmd, name, DateTime)NpgsqlDbType.Timestamp, so.With(name, DateTime.UtcNow)always threwThe second and third were found while fixing the first; each would have left the bug reachable.
Column types are deliberately untouched
A column has one type, so DDL stays per-type —
GetDatabaseType(typeof(DateTime))still returnstimestamp without time zone, now pinned by a test. Only parameters follow their value.Verification
Live Postgres, every case previously throwing:
DateTime.UtcNowTimestampTz✓DateTime[]all UtcArray | TimestampTz✓List<DateTime>all UtcArray | TimestampTz✓DateTime.NowTimestamp✓Timestamp✓DateTime[]emptyArray | Timestamp✓string/Guid/int/DateTimeOffset/string[]Includes an integration test that actually writes and reads back a UTC
DateTime, since the unit-level assertion alone would not have caught this.Suites: Postgres 798 passed on all four matrix legs (net9.0/net10.0 × case-sensitive true/false), Core 21, SQLite 361. MSSQL / Oracle / MySQL covered by CI — unchanged by the default implementation.
On #404
This removes the observable
AddParameter/AddNamedParameterdivergence forDateTime: both now yieldTimestampTzfor a UTC value, one by deferring to Npgsql and one by resolving per value. What remains of #404 is narrower —AddNamedParameterstill throws for types with no mapping whereAddParametersucceeds. Left open deliberately; it is an API-direction call, not a bug fix.🤖 Generated with Claude Code