Skip to content

AddParameter and AddNamedParameter disagree on how to type an untyped value #404

Description

@jeremydmiller

Split out of #398 / #402. Closely tied to #403.

Given no explicit dbType, the two sibling APIs on DatabaseProvider do opposite things:

// AddParameter -- stamps nothing, defers to the driver
parameter.Value = value ?? DBNull.Value;
if (dbType.HasValue) { SetParameterType(parameter, dbType.Value); }

// AddNamedParameter -- stamps Weasel's own CLR-type mapping
if (dbType.HasValue)      { SetParameterType(parameter, dbType.Value); }
else if (value != null)   { SetParameterType(parameter, ToParameterType(value.GetType())); }

Observable difference:

AddParameter(cmd, DateTime.UtcNow)           -> TimestampTz  (Npgsql's per-value choice)
AddNamedParameter(cmd, "n", DateTime.UtcNow) -> Timestamp    (Weasel's per-type mapping) -> throws

Two knock-on effects:

  1. AddNamedParameter sends a UTC DateTime as 'timestamp without time zone' and throws at execution #403 — the stamping path is wrong for UTC DateTime.
  2. Flaky: string parameters can resolve to NpgsqlDbType.Unknown (add_first_parameter failed 3/4 CI jobs, green on re-run) #398 — the deferring path means param.NpgsqlDbType reports Unknown until Npgsql's process-global type mapper has been seeded, which made a test flaky.

These want a single deliberate answer rather than two accidental ones. Roughly:

  • Defer everywhere. Drop the ToParameterType call so both match AddParameter. Fixes AddNamedParameter sends a UTC DateTime as 'timestamp without time zone' and throws at execution #403 for free and is closest to what each ADO.NET driver does best, since drivers resolve per value. Changes behaviour for SQL Server / Oracle / MySQL / SQLite too, so it needs checking that their inference matches Weasel's mapping the way Npgsql's does.
  • Stamp everywhere, per value. Give providers a value-aware hook so PostgreSQL can answer timestamptz vs timestamp from DateTime.Kind. Keeps types explicit but duplicates driver logic in Weasel.

Note ToParameterType also throws for types with no mapping (and for arrays on SQL Server / Oracle / MySQL), so today AddNamedParameter can throw where AddParameter succeeds. Whichever direction is chosen should settle that too.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions