fix(sql): bind string keys untyped on Postgres so integer key columns compare (#602 STAB-24)

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-08 07:03:00 -04:00
parent 5a5c80ee57
commit 71881163df
8 changed files with 161 additions and 0 deletions
@@ -107,6 +107,21 @@ public interface ISqlDialect
    /// <param name="command">The command to configure.</param>
    void ConfigureCommand(DbCommand command);

+    /// <summary>
+    ///     Gives the dialect a chance to adjust a poll-path parameter after its name and value are set and
+    ///     before it is added to the command (STAB-24). Authored key / row-selector values are CLR strings,
+    ///     because a tag's <c>keyValue</c> is configuration text — which a strictly-typed provider will not
+    ///     compare against a numeric key column (Npgsql sends <c>text</c>, and
+    ///     <c>integer = text</c> has no operator). A dialect that needs the server to infer the type from
+    ///     context overrides this. The default does nothing, which is correct for providers that coerce
+    ///     implicitly (SQL Server, ODP.NET).
+    ///     <para>Implementations must tolerate any <see cref="DbParameter"/> they are handed, for the same
+    ///     substituted-factory reason as <see cref="ConfigureCommand"/>.</para>
+    /// </summary>
+    /// <param name="parameter">The parameter, with <see cref="DbParameter.ParameterName"/> and value already set.</param>
+    /// <param name="value">The authored value being bound (never <see cref="DBNull"/>; may be <c>null</c>).</param>
+    void ConfigureParameter(DbParameter parameter, object? value) { }
+
    /// <summary>
    ///     The most bound parameters this backend accepts in <b>one</b> command — 2098 usable of SQL Server's
    ///     2100, 1000 entries in an Oracle <c>IN</c> expression list, 65535 on the Postgres wire protocol.
@@ -64,6 +64,21 @@ public sealed class PostgresDialect : ISqlDialect
        // Intentionally empty: see the summary.
    }

+    /// <summary>
+    ///     STAB-24: a string-valued bind is sent with <see cref="NpgsqlTypes.NpgsqlDbType.Unknown"/> so the
+    ///     server infers its type from the compared column. Bound as the default <c>text</c>, a tag whose
+    ///     <c>keyValue</c> is <c>"3"</c> against an <c>integer</c> key column fails with
+    ///     <c>operator does not exist: integer = text</c>. Unknown still carries the value as data — it is
+    ///     never interpolated — so the injection boundary is unchanged.
+    /// </summary>
+    /// <param name="parameter">The parameter to adjust.</param>
+    /// <param name="value">The authored value.</param>
+    public void ConfigureParameter(DbParameter parameter, object? value)
+    {
+        if (parameter is NpgsqlParameter npgsql && value is string)
+            npgsql.NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Unknown;
+    }
+
    /// <summary>
    ///     65535 — the extended-query protocol carries the parameter count as a signed/unsigned 16-bit field
    ///     in <c>Bind</c>, so that is a protocol ceiling rather than a server setting, and Npgsql raises it
@@ -405,6 +405,7 @@ public sealed class SqlPollReader
                        // and never from ParameterNames: the marker and the bind name differ on ODP.NET.
                        parameter.ParameterName = plan.ParameterBindNames[i];
                        parameter.Value = plan.Parameters[i] ?? DBNull.Value;
+                        _dialect.ConfigureParameter(parameter, plan.Parameters[i]);
                        command.Parameters.Add(parameter);
                    }

@@ -80,6 +80,20 @@ VALUES (7, 180.5,  1.2, TIMESTAMP '2026-07-24 10:00:00'),
       (9, NULL,   1.6, TIMESTAMP '2026-07-24 10:00:01'),
       (8, 210.25, 1.4, TIMESTAMP '2026-07-24 10:00:02');

+-- Integer-keyed EAV source (STAB-24): the key column is `integer`, so a key bound as the default
+-- `text` fails with "operator does not exist: integer = text". Pins the untyped-bind fix in
+-- PostgresDialect.ConfigureParameter for the key-value IN-list path (the wide-row `whereColumn`
+-- path is covered by latest_status.station_id above).
+CREATE TABLE sqlpoll.int_tag_values (
+    tag_id    integer          NOT NULL,
+    num_value double precision NULL,
+    sample_ts timestamp(3)     NOT NULL
+);
+
+INSERT INTO sqlpoll.int_tag_values (tag_id, num_value, sample_ts)
+VALUES (101, 77.5, TIMESTAMP '2026-07-24 10:00:00'),
+       (102, 12.0, TIMESTAMP '2026-07-24 10:00:01');
+
 RESET ROLE;

 -- ── grants: USAGE + SELECT and nothing else ───────────────────────────────────────────────────
@@ -114,6 +114,63 @@ public sealed class PostgresReadTests
            .ShouldBe(PostgresFixture.NewestStationOvenTemp);
    }

+    // ---- integer key columns (STAB-24) ----
+
+    /// <summary>
+    ///     A key-value tag whose key column is <c>integer</c> reads. Authored <c>keyValue</c> is text, and
+    ///     Npgsql's default <c>text</c> bind has no <c>integer = text</c> operator — the read failed with a
+    ///     provider error before <c>PostgresDialect.ConfigureParameter</c> sent string binds untyped.
+    /// </summary>
+    [Fact]
+    public async Task ReadAsync_realPostgres_integerKeyColumnComparesAgainstAuthoredText()
+    {
+        SkipUnlessLive();
+
+        await using var driver = await StartAsync(Tag("Sql/IntKey/Tag101", new JsonObject
+        {
+            ["driver"] = "Sql",
+            ["model"] = "KeyValue",
+            ["table"] = PostgresFixture.Qualified("int_tag_values"),
+            ["keyColumn"] = "tag_id",
+            ["keyValue"] = "101",
+            ["valueColumn"] = PostgresFixture.ValueColumn,
+            ["timestampColumn"] = PostgresFixture.TimestampColumn,
+        }));
+
+        var snapshot = (await driver.ReadAsync(["Sql/IntKey/Tag101"], TestContext.Current.CancellationToken))
+            .ShouldHaveSingleItem();
+
+        SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue();
+        Convert.ToDouble(snapshot.Value, CultureInfo.InvariantCulture).ShouldBe(77.5);
+    }
+
+    /// <summary>A wide-row <c>whereColumn</c> selector over the <c>integer station_id</c> column reads the matching row.</summary>
+    [Fact]
+    public async Task ReadAsync_realPostgres_integerWhereColumnSelectsTheMatchingStation()
+    {
+        SkipUnlessLive();
+
+        await using var driver = await StartAsync(Tag("Sql/Station7/OvenTemp", new JsonObject
+        {
+            ["driver"] = "Sql",
+            ["model"] = "WideRow",
+            ["table"] = PostgresFixture.Qualified(PostgresFixture.WideRowTable),
+            ["columnName"] = "oven_temp",
+            ["timestampColumn"] = PostgresFixture.TimestampColumn,
+            ["rowSelector"] = new JsonObject
+            {
+                ["whereColumn"] = "station_id",
+                ["whereValue"] = "7",
+            },
+        }));
+
+        var snapshot = (await driver.ReadAsync(["Sql/Station7/OvenTemp"], TestContext.Current.CancellationToken))
+            .ShouldHaveSingleItem();
+
+        SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue();
+        Convert.ToDouble(snapshot.Value, CultureInfo.InvariantCulture).ShouldBe(180.5);
+    }
+
    // ---- catalog case-folding (design §8.1 gate, against a real information_schema) ----

    /// <summary>
@@ -104,6 +104,16 @@ public sealed class ColonMarkerDialect : ISqlDialect
        lock (_commands) _commands.Add(command);
    }

+    /// <summary>Every value handed to <see cref="ConfigureParameter"/>, in call order.</summary>
+    public List<object?> ConfiguredParameterValues { get; } = [];
+
+    /// <inheritdoc/>
+    public void ConfigureParameter(DbParameter parameter, object? value)
+    {
+        ArgumentNullException.ThrowIfNull(parameter);
+        lock (ConfiguredParameterValues) ConfiguredParameterValues.Add(value);
+    }
+
    /// <inheritdoc/>
    public string QuoteIdentifier(string ident) => Inner.QuoteIdentifier(ident);

@@ -24,6 +24,26 @@ public class PostgresDialectTests
        d.QuoteIdentifier("a\"b").ShouldBe("\"a\"\"b\"");
    }

+    /// <summary>STAB-24: a string bind is untyped so Postgres infers it against an integer key column.</summary>
+    [Fact]
+    public void ConfigureParameter_sendsStringBindsAsUnknownType_soIntegerKeysCompare()
+    {
+        var p = new Npgsql.NpgsqlParameter("@k0", "3") { NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Text };
+        new PostgresDialect().ConfigureParameter(p, "3");
+        p.NpgsqlDbType.ShouldBe(NpgsqlTypes.NpgsqlDbType.Unknown);
+    }
+
+    /// <summary>Non-string values keep their natural type, and a foreign parameter type is tolerated.</summary>
+    [Fact]
+    public void ConfigureParameter_leavesNonStringsAndForeignParametersAlone()
+    {
+        var d = new PostgresDialect();
+        var n = new Npgsql.NpgsqlParameter("@k0", 3) { NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Integer };
+        d.ConfigureParameter(n, 3);
+        n.NpgsqlDbType.ShouldBe(NpgsqlTypes.NpgsqlDbType.Integer);
+        Should.NotThrow(() => d.ConfigureParameter(new SqliteParameter("@k0", "3"), "3"));
+    }
+
    [Fact]
    public void QuoteIdentifier_rejectsControlCharsAndNul()
        => Should.Throw<ArgumentException>(() => new PostgresDialect().QuoteIdentifier("x\0y"));
@@ -318,6 +318,35 @@ public sealed class SqlCatalogGateDriverTests
        dialect.ConfiguredCommands.ShouldBeGreaterThan(0);
    }

+    /// <summary>
+    ///     STAB-24: every poll-path bind goes through <see cref="ISqlDialect.ConfigureParameter"/> with the
+    ///     authored value, so a dialect can type it. Without the call the Postgres untyped-bind fix is inert.
+    /// </summary>
+    [Fact]
+    public async Task The_poll_binds_every_authored_value_through_the_dialects_parameter_hook()
+    {
+        using var fixture = new SqlitePollFixture();
+        var dialect = new ColonMarkerDialect();
+        await using var driver = new SqlDriver(
+            new SqlDriverOptions
+            {
+                RawTags = [KvEntry("Speed", SqlitePollFixture.PresentKey)],
+                OperationTimeout = TimeSpan.FromSeconds(15),
+                CommandTimeout = TimeSpan.FromSeconds(10),
+            },
+            DriverInstanceId,
+            dialect,
+            fixture.ConnectionString,
+            factory: fixture.Factory,
+            logger: NullLogger<SqlDriver>.Instance);
+        await driver.InitializeAsync(ConfigJson, CancellationToken.None);
+
+        var snapshots = await driver.ReadAsync(["Speed"], CancellationToken.None);
+
+        SqlStatusCodes.IsGood(snapshots[0].StatusCode).ShouldBeTrue();
+        dialect.ConfiguredParameterValues.ShouldContain(SqlitePollFixture.PresentKey);
+    }
+
    // ---- helpers ----

    /// <summary>