From 71881163dff9b5f71515c5342fd78c2e511a3446 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 07:03:00 -0400 Subject: [PATCH] fix(sql): bind string keys untyped on Postgres so integer key columns compare (#602 STAB-24) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../ISqlDialect.cs | 15 +++++ .../PostgresDialect.cs | 15 +++++ .../SqlPollReader.cs | 1 + .../Docker/postgres/seed-pg.sql | 14 +++++ .../PostgresReadTests.cs | 57 +++++++++++++++++++ .../ColonMarkerDialect.cs | 10 ++++ .../PostgresDialectTests.cs | 20 +++++++ .../SqlCatalogGateDriverTests.cs | 29 ++++++++++ 8 files changed, 161 insertions(+) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs index d968389e..40ba4ec9 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs @@ -107,6 +107,21 @@ public interface ISqlDialect /// The command to configure. void ConfigureCommand(DbCommand command); + /// + /// 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 keyValue is configuration text — which a strictly-typed provider will not + /// compare against a numeric key column (Npgsql sends text, and + /// integer = text 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). + /// Implementations must tolerate any they are handed, for the same + /// substituted-factory reason as . + /// + /// The parameter, with and value already set. + /// The authored value being bound (never ; may be null). + void ConfigureParameter(DbParameter parameter, object? value) { } + /// /// The most bound parameters this backend accepts in one command — 2098 usable of SQL Server's /// 2100, 1000 entries in an Oracle IN expression list, 65535 on the Postgres wire protocol. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs index 8457d1c2..f52cf8d4 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs @@ -64,6 +64,21 @@ public sealed class PostgresDialect : ISqlDialect // Intentionally empty: see the summary. } + /// + /// STAB-24: a string-valued bind is sent with so the + /// server infers its type from the compared column. Bound as the default text, a tag whose + /// keyValue is "3" against an integer key column fails with + /// operator does not exist: integer = text. Unknown still carries the value as data — it is + /// never interpolated — so the injection boundary is unchanged. + /// + /// The parameter to adjust. + /// The authored value. + public void ConfigureParameter(DbParameter parameter, object? value) + { + if (parameter is NpgsqlParameter npgsql && value is string) + npgsql.NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Unknown; + } + /// /// 65535 — the extended-query protocol carries the parameter count as a signed/unsigned 16-bit field /// in Bind, so that is a protocol ceiling rather than a server setting, and Npgsql raises it diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs index 8e17407b..6e572637 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs @@ -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); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/postgres/seed-pg.sql b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/postgres/seed-pg.sql index 21c1b520..59ce06da 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/postgres/seed-pg.sql +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/postgres/seed-pg.sql @@ -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 ─────────────────────────────────────────────────── diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/PostgresReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/PostgresReadTests.cs index fcf8f4a4..e9bc7fd3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/PostgresReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/PostgresReadTests.cs @@ -114,6 +114,63 @@ public sealed class PostgresReadTests .ShouldBe(PostgresFixture.NewestStationOvenTemp); } + // ---- integer key columns (STAB-24) ---- + + /// + /// A key-value tag whose key column is integer reads. Authored keyValue is text, and + /// Npgsql's default text bind has no integer = text operator — the read failed with a + /// provider error before PostgresDialect.ConfigureParameter sent string binds untyped. + /// + [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); + } + + /// A wide-row whereColumn selector over the integer station_id column reads the matching row. + [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) ---- /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs index fa8dd2b3..f514b1be 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs @@ -104,6 +104,16 @@ public sealed class ColonMarkerDialect : ISqlDialect lock (_commands) _commands.Add(command); } + /// Every value handed to , in call order. + public List ConfiguredParameterValues { get; } = []; + + /// + public void ConfigureParameter(DbParameter parameter, object? value) + { + ArgumentNullException.ThrowIfNull(parameter); + lock (ConfiguredParameterValues) ConfiguredParameterValues.Add(value); + } + /// public string QuoteIdentifier(string ident) => Inner.QuoteIdentifier(ident); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs index bfd979f8..2f56b7d1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs @@ -24,6 +24,26 @@ public class PostgresDialectTests d.QuoteIdentifier("a\"b").ShouldBe("\"a\"\"b\""); } + /// STAB-24: a string bind is untyped so Postgres infers it against an integer key column. + [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); + } + + /// Non-string values keep their natural type, and a foreign parameter type is tolerated. + [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(() => new PostgresDialect().QuoteIdentifier("x\0y")); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs index f94714c3..f0e5d373 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs @@ -318,6 +318,35 @@ public sealed class SqlCatalogGateDriverTests dialect.ConfiguredCommands.ShouldBeGreaterThan(0); } + /// + /// STAB-24: every poll-path bind goes through with the + /// authored value, so a dialect can type it. Without the call the Postgres untyped-bind fix is inert. + /// + [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.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 ---- ///