refactor(sql): dialect-owned parameter markers, bind names and command configuration

ISqlDialect gains ParameterMarker (the token in the SQL text), ParameterBindName
(the DbParameter.ParameterName) and ConfigureCommand (per-command provider setup,
where Oracle will set BindByName). SQL Server and the test SQLite dialect answer
"@"+name for both spellings and no-op the hook, so SQL Server behaviour is
byte-identical — every existing expected-SQL assertion passes untouched.

SqlGroupPlanner drops its hardcoded @k/@w consts and mints base names (k0..kN, w)
that the dialect spells; SqlQueryPlan carries the emitted markers AND the matching
bind names, so SqlPollReader keeps copying the plan's pairing rather than
re-deriving a provider convention (the two diverge on ODP.NET: ":k0" in the text,
bare "k0" on the parameter). SqlCatalogLoader and SqlBrowseSession bind
schema/table through the dialect, so their bind names cannot drift from the
markers in the dialect-owned catalog SQL. Every parameter-carrying command site
calls ConfigureCommand before its first parameter.

Controlled by a new test-only ColonMarkerDialect (":name" markers, bare bind
names) that really executes against the SQLite fixture. Each leg was falsified
against the pre-change behaviour: the planner emitted "@k0", the reader bound
":k0", and a hardcoded "@schema" in the loader failed catalog load outright.
This commit is contained in:
Joseph Doherty
2026-08-05 06:25:53 -04:00
parent 08c1728a98
commit 1da770253e
16 changed files with 527 additions and 36 deletions
@@ -14,8 +14,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// tables issues a handful of round-trips at Initialize and none thereafter; it never enumerates a
/// warehouse.</para>
/// <para><b>Every parameter is bound.</b> Authored names reach the catalog queries as
/// <c>@schema</c>/<c>@table</c> parameters — the same discipline the schema browser follows — so loading
/// the allow-list cannot itself be an injection vector. No identifier is quoted into text on this path.</para>
/// <c>schema</c>/<c>table</c> parameters — the same discipline the schema browser follows — so loading
/// the allow-list cannot itself be an injection vector. No identifier is quoted into text on this path.
/// The bind names come from <see cref="ISqlDialect.ParameterBindName"/> over those two base names, so
/// they can never disagree with the markers in the dialect's own catalog SQL (<c>@schema</c> on SQL
/// Server, <c>:schema</c> on Oracle).</para>
/// <para><b>Failures throw; they do not degrade to an empty catalog.</b> An empty catalog would reject
/// every tag, which is indistinguishable at the OPC UA surface from a database whose objects were all
/// dropped. A caller that cannot read the catalog has not learned that the tags are invalid — it has
@@ -33,6 +36,12 @@ internal static class SqlCatalogLoader
/// <summary>Column alias <see cref="ISqlDialect.ListColumnsSql"/> projects.</summary>
private const string ColumnNameColumn = "COLUMN_NAME";
/// <summary>Base name of the schema parameter every dialect's catalog SQL carries.</summary>
private const string SchemaParameter = "schema";
/// <summary>Base name of the table parameter <see cref="ISqlDialect.ListColumnsSql"/> carries.</summary>
private const string TableParameter = "table";
/// <summary>
/// Reads the catalog slice covering <paramref name="authoredTables"/>.
/// </summary>
@@ -53,7 +62,7 @@ internal static class SqlCatalogLoader
var timeoutSeconds = Math.Max(1, (int)Math.Ceiling(commandTimeout.TotalSeconds));
var schemas = await QueryColumnAsync(
connection, dialect.ListSchemasSql, SchemaColumn,
connection, dialect, dialect.ListSchemasSql, SchemaColumn,
static _ => { }, timeoutSeconds, cancellationToken).ConfigureAwait(false);
// Zero schemas is a permissions or visibility symptom, never a real database. Rejecting every tag on
@@ -94,8 +103,8 @@ internal static class SqlCatalogLoader
if (!tablesBySchema.TryGetValue(schema, out var tables))
{
tables = await QueryColumnAsync(
connection, dialect.ListTablesSql, TableNameColumn,
command => Bind(command, "@schema", schema),
connection, dialect, dialect.ListTablesSql, TableNameColumn,
command => Bind(command, dialect.ParameterBindName(SchemaParameter), schema),
timeoutSeconds, cancellationToken).ConfigureAwait(false);
tablesBySchema[schema] = tables;
}
@@ -106,11 +115,11 @@ internal static class SqlCatalogLoader
if (columnsByTable.ContainsKey(key)) continue;
columnsByTable[key] = await QueryColumnAsync(
connection, dialect.ListColumnsSql, ColumnNameColumn,
connection, dialect, dialect.ListColumnsSql, ColumnNameColumn,
command =>
{
Bind(command, "@schema", schema);
Bind(command, "@table", table);
Bind(command, dialect.ParameterBindName(SchemaParameter), schema);
Bind(command, dialect.ParameterBindName(TableParameter), table);
},
timeoutSeconds, cancellationToken).ConfigureAwait(false);
}
@@ -133,6 +142,7 @@ internal static class SqlCatalogLoader
var command = connection.CreateCommand();
await using (command.ConfigureAwait(false))
{
dialect.ConfigureCommand(command);
command.CommandText = dialect.DefaultSchemaSql;
command.CommandTimeout = timeoutSeconds;
var value = await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false);
@@ -144,6 +154,7 @@ internal static class SqlCatalogLoader
/// <summary>Runs one catalog query and projects a single string column from every row.</summary>
private static async Task<IReadOnlyList<string>> QueryColumnAsync(
DbConnection connection,
ISqlDialect dialect,
string sql,
string columnName,
Action<DbCommand> bind,
@@ -155,6 +166,8 @@ internal static class SqlCatalogLoader
var command = connection.CreateCommand();
await using (command.ConfigureAwait(false))
{
// Before the binds: ODP.NET's BindByName has to be set while the parameter collection is empty.
dialect.ConfigureCommand(command);
command.CommandText = sql;
command.CommandTimeout = timeoutSeconds;
bind(command);
@@ -134,6 +134,9 @@ public sealed class SqlDriverProbe : IDriverProbe
var command = connection.CreateCommand();
await using (command.ConfigureAwait(false))
{
// Uniformly applied at every command site, even where this statement takes no
// parameters — a provider hook that is only *sometimes* honoured is the harder bug.
dialect.ConfigureCommand(command);
command.CommandText = dialect.LivenessSql;
await command.ExecuteScalarAsync(cts.Token).ConfigureAwait(false);
}
@@ -13,16 +13,22 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// <c>SqlGroupPlannerTests</c>.</para>
/// <para><b>Injection boundary (design §8.1).</b> Identifiers reach the command text through
/// <see cref="ISqlDialect.QuoteIdentifier"/> and nothing else; every authored value becomes a
/// <c>@k0</c>/<c>@w</c> marker with the value carried in <see cref="SqlQueryPlan.Parameters"/>. There is no
/// interpolation of tag content into SQL anywhere in this type.</para>
/// <c>k0</c>/<c>w</c> parameter — spelled with the dialect's own <see cref="ISqlDialect.ParameterMarker"/>
/// (<c>@k0</c> on SQL Server, <c>:k0</c> on Oracle) — with the value carried in
/// <see cref="SqlQueryPlan.Parameters"/>. There is no interpolation of tag content into SQL anywhere in
/// this type.</para>
/// <para><b>The base names are this planner's, the spelling is the dialect's.</b> Each plan carries the
/// markers it emitted <em>and</em> the matching <see cref="ISqlDialect.ParameterBindName"/>s, so the
/// reader binds what the planner minted instead of re-deriving a provider convention — the two differ on
/// ODP.NET (<c>:k0</c> in the text, bare <c>k0</c> on the parameter).</para>
/// </summary>
public static class SqlGroupPlanner
{
/// <summary>The parameter-marker prefix for a key-value model's <c>IN</c> list.</summary>
private const string KeyMarkerPrefix = "@k";
/// <summary>The base-name prefix for a key-value model's <c>IN</c>-list parameters (<c>k0…kN</c>).</summary>
private const string KeyBaseNamePrefix = "k";
/// <summary>The single parameter marker for a wide-row model's row selector.</summary>
private const string RowSelectorMarker = "@w";
/// <summary>The single parameter base name for a wide-row model's row selector.</summary>
private const string RowSelectorBaseName = "w";
/// <summary>
/// Groups <paramref name="tags"/> by source and emits one <see cref="SqlQueryPlan"/> per group.
@@ -132,7 +138,8 @@ public static class SqlGroupPlanner
if (timestampColumn is not null) AddDistinct(selected, timestampColumn);
// A keyValue may legitimately be the empty string, so only null is a broken invariant.
var names = new List<string>(members.Count);
var markers = new List<string>(members.Count);
var bindNames = new List<string>(members.Count);
var values = new List<object?>(members.Count);
var seen = new HashSet<string>(StringComparer.Ordinal);
foreach (var member in members)
@@ -140,7 +147,9 @@ public static class SqlGroupPlanner
var keyValue = member.KeyValue
?? throw MissingField(nameof(SqlTagDefinition.KeyValue), member);
if (!seen.Add(keyValue)) continue;
names.Add(KeyMarkerPrefix + values.Count.ToString(CultureInfo.InvariantCulture));
var baseName = KeyBaseNamePrefix + values.Count.ToString(CultureInfo.InvariantCulture);
markers.Add(dialect.ParameterMarker(baseName));
bindNames.Add(dialect.ParameterBindName(baseName));
values.Add(keyValue);
}
@@ -148,10 +157,11 @@ public static class SqlGroupPlanner
.Append(QuoteList(selected, dialect))
.Append(" FROM ").Append(QuoteQualifiedName(first.Table, dialect))
.Append(" WHERE ").Append(dialect.QuoteIdentifier(keyColumn))
.Append(" IN (").AppendJoin(", ", names).Append(')')
.Append(" IN (").AppendJoin(", ", markers).Append(')')
.ToString();
return new SqlQueryPlan(SqlTagModel.KeyValue, groupKey, sql, names, values, members, selected);
return new SqlQueryPlan(
SqlTagModel.KeyValue, groupKey, sql, markers, bindNames, values, members, selected);
}
/// <summary>
@@ -183,12 +193,14 @@ public static class SqlGroupPlanner
// A where-pair wins over topByTimestamp; the parser already guarantees exactly one is populated.
if (!Blank(first.RowSelectorColumn) && first.RowSelectorValue is not null)
{
var marker = dialect.ParameterMarker(RowSelectorBaseName);
var sql = string.Concat(
"SELECT ", columns, " FROM ", table,
" WHERE ", dialect.QuoteIdentifier(first.RowSelectorColumn!), " = ", RowSelectorMarker);
" WHERE ", dialect.QuoteIdentifier(first.RowSelectorColumn!), " = ", marker);
return new SqlQueryPlan(
SqlTagModel.WideRow, groupKey, sql,
[RowSelectorMarker], [first.RowSelectorValue], members, selected);
[marker], [dialect.ParameterBindName(RowSelectorBaseName)],
[first.RowSelectorValue], members, selected);
}
if (!Blank(first.RowSelectorTopByTimestamp))
@@ -200,7 +212,7 @@ public static class SqlGroupPlanner
"SELECT ", dialect.SingleRowLimitPrefix, columns, " FROM ", table,
" ORDER BY ", dialect.QuoteIdentifier(first.RowSelectorTopByTimestamp!), " DESC",
dialect.SingleRowLimitSuffix);
return new SqlQueryPlan(SqlTagModel.WideRow, groupKey, sql, [], [], members, selected);
return new SqlQueryPlan(SqlTagModel.WideRow, groupKey, sql, [], [], [], members, selected);
}
throw new ArgumentException(
@@ -393,13 +393,17 @@ public sealed class SqlPollReader
var command = connection.CreateCommand();
await using (command.ConfigureAwait(false))
{
// Before any parameter is added: on ODP.NET this is what turns bind-by-position off, and
// a positionally-bound IN list feeds one tag's value to another tag's node in silence.
_dialect.ConfigureCommand(command);
command.CommandText = plan.SqlText;
command.CommandTimeout = _commandTimeoutSeconds;
for (var i = 0; i < plan.ParameterNames.Count; i++)
for (var i = 0; i < plan.ParameterBindNames.Count; i++)
{
var parameter = command.CreateParameter();
// Bound by the plan's own index pairing — never by re-deriving the marker convention.
parameter.ParameterName = plan.ParameterNames[i];
// Bound by the plan's own index pairing — never by re-deriving the marker convention,
// 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;
command.Parameters.Add(parameter);
}
@@ -9,8 +9,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// per group rather than one query per tag.
/// <para><b>Injection boundary.</b> <see cref="SqlText"/> is built from dialect-quoted identifiers only;
/// every authored <em>value</em> lives in <see cref="Parameters"/> and is bound as a
/// <see cref="DbParameter"/> named by the positionally-aligned <see cref="ParameterNames"/>. There is no
/// path by which a tag's <c>keyValue</c> or <c>whereValue</c> reaches the command text.</para>
/// <see cref="DbParameter"/> named by the positionally-aligned <see cref="ParameterBindNames"/>. There is
/// no path by which a tag's <c>keyValue</c> or <c>whereValue</c> reaches the command text.</para>
/// <para><b>Markers and bind names are carried separately</b> because they are not the same string on
/// every provider: ODP.NET's text says <c>:k0</c> while its <see cref="DbParameter.ParameterName"/> is the
/// bare <c>k0</c>. Both lists are positionally aligned with <see cref="Parameters"/>, so a binding site
/// copies the pairing the planner minted rather than re-deriving a provider convention it does not own.</para>
/// <para>Produced only by <see cref="SqlGroupPlanner.Plan"/>; consumed by the poll reader, which executes
/// <see cref="SqlText"/> once and slices the rows back to per-member snapshots.</para>
/// </summary>
@@ -30,9 +34,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// with <see cref="Parameters"/>. Carried explicitly so the reader never has to re-derive the naming
/// convention.
/// </param>
/// <param name="ParameterBindNames">
/// The <see cref="DbParameter.ParameterName"/> to bind each value under, from
/// <see cref="ISqlDialect.ParameterBindName"/>, aligned index-for-index with <see cref="Parameters"/> and
/// with <see cref="ParameterNames"/>. Equal to the marker on every provider but ODP.NET, which wants the
/// bare name — which is exactly why it is a carried list and not a transformation of the marker.
/// </param>
/// <param name="Parameters">
/// The values to bind, aligned with <see cref="ParameterNames"/>. Captured verbatim from the authored tags
/// — a hostile value is inert here because it is data, not text.
/// The values to bind, aligned with <see cref="ParameterNames"/> and <see cref="ParameterBindNames"/>.
/// Captured verbatim from the authored tags — a hostile value is inert here because it is data, not text.
/// </param>
/// <param name="Members">
/// The tags this plan feeds, in the planner's input order. Never empty. Duplicates are preserved: two tags
@@ -47,6 +57,7 @@ public sealed record SqlQueryPlan(
object GroupKey,
string SqlText,
IReadOnlyList<string> ParameterNames,
IReadOnlyList<string> ParameterBindNames,
IReadOnlyList<object?> Parameters,
IReadOnlyList<SqlTagDefinition> Members,
IReadOnlyList<string> SelectedColumns)
@@ -57,6 +68,9 @@ public sealed record SqlQueryPlan(
/// <summary>The parameter markers in <see cref="SqlText"/>, aligned with <see cref="Parameters"/>.</summary>
public IReadOnlyList<string> ParameterNames { get; } = [.. ParameterNames];
/// <summary>The parameter names to bind under, aligned with <see cref="Parameters"/>.</summary>
public IReadOnlyList<string> ParameterBindNames { get; } = [.. ParameterBindNames];
/// <summary>The values to bind, aligned with <see cref="ParameterNames"/>.</summary>
public IReadOnlyList<object?> Parameters { get; } = [.. Parameters];