fix(sql): row-cap key-value and where-pair plans with a TOP/LIMIT N+1 dialect seam (R06:S-1)

Key-value and where-pair plans had no row limit, so a selector pointed at a history
or events table materialised every matching row on every poll. ISqlDialect gains
RowLimitPrefix/RowLimitSuffix(n) (SQL Server TOP (n), Postgres LIMIT n, Oracle FETCH
FIRST n ROWS ONLY); plans carry a RowCap (distinct keys, or 1) and ask for RowCap+1
rows; the reader also stops client-side at RowCap+1. Getting the extra row proves the
selector ambiguous: the group publishes BadConfigurationError with a rate-limited
warning. The reader reports those answered refusals so the driver counts them as
answer evidence, and any refusal-only poll now restores Healthy after a comms fault.

Refs #744
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 15:57:13 -04:00
parent 7f5d959fb9
commit 0a5aa9fc43
21 changed files with 420 additions and 54 deletions
@@ -177,6 +177,31 @@ public interface ISqlDialect
/// </summary>
string SingleRowLimitSuffix { get; }
/// <summary>
/// The <see cref="SingleRowLimitPrefix"/> position's fragment for an <paramref name="rows"/>-row limit —
/// T-SQL's <c>"TOP (n) "</c>; empty for a dialect that limits at the end. Same trailing-space rule.
/// </summary>
/// <remarks>
/// Added for the key-value / where-pair row cap (R06:S-1): those plans used to carry no limit, so an
/// ambiguous selector against a history table materialised every matching row on every poll. The
/// planner asks for <c>N + 1</c> rows — one more than the plan can use — so ambiguity stays observable
/// (the reader sees the extra row and publishes a configuration error) without unbounded reads.
/// </remarks>
/// <param name="rows">The row limit; at least 1.</param>
/// <returns>The fragment, possibly empty.</returns>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="rows"/> is below 1.</exception>
string RowLimitPrefix(int rows);
/// <summary>
/// The <see cref="SingleRowLimitSuffix"/> position's fragment for an <paramref name="rows"/>-row limit —
/// <c>" LIMIT n"</c> / <c>" FETCH FIRST n ROWS ONLY"</c>; empty for a dialect that limits after
/// <c>SELECT</c>. Same leading-space rule. See <see cref="RowLimitPrefix"/>.
/// </summary>
/// <param name="rows">The row limit; at least 1.</param>
/// <returns>The fragment, possibly empty.</returns>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="rows"/> is below 1.</exception>
string RowLimitSuffix(int rows);
/// <summary>Catalog query listing schemas — the browser's <c>RootAsync</c> level. Takes no parameters.</summary>
string ListSchemasSql { get; }
@@ -124,6 +124,21 @@ public sealed class OracleDialect : ISqlDialect
// statement shape, which this seam deliberately leaves with the planner.
public string SingleRowLimitSuffix => " FETCH FIRST 1 ROWS ONLY";
/// <inheritdoc />
public string RowLimitPrefix(int rows)
{
ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1);
return string.Empty;
}
/// <inheritdoc />
// The same 12c+ row-limiting clause as SingleRowLimitSuffix; valid after a WHERE with no ORDER BY.
public string RowLimitSuffix(int rows)
{
ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1);
return " FETCH FIRST " + rows.ToString(System.Globalization.CultureInfo.InvariantCulture) + " ROWS ONLY";
}
/// <inheritdoc />
// Schemas visible to the connecting principal, from ALL_USERS (Oracle equates a schema with a
// user), aliased to the TABLE_SCHEMA column name the catalog readers project by.
@@ -86,6 +86,20 @@ public sealed class PostgresDialect : ISqlDialect
/// <inheritdoc />
public string SingleRowLimitSuffix => " LIMIT 1";
/// <inheritdoc />
public string RowLimitPrefix(int rows)
{
ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1);
return string.Empty;
}
/// <inheritdoc />
public string RowLimitSuffix(int rows)
{
ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1);
return " LIMIT " + rows.ToString(System.Globalization.CultureInfo.InvariantCulture);
}
// The schemas holding relations this connection can actually see, system schemas excluded.
//
// Sourced from information_schema.tables, not information_schema.schemata - a deliberate
@@ -87,6 +87,22 @@ public sealed class SqlServerDialect : ISqlDialect
/// <inheritdoc />
public string SingleRowLimitSuffix => string.Empty;
/// <inheritdoc />
// Parenthesised: TOP (n) is the documented modern form, and TOP n without parentheses is the
// backwards-compatibility spelling.
public string RowLimitPrefix(int rows)
{
ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1);
return "TOP (" + rows.ToString(System.Globalization.CultureInfo.InvariantCulture) + ") ";
}
/// <inheritdoc />
public string RowLimitSuffix(int rows)
{
ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1);
return string.Empty;
}
/// <inheritdoc/>
public string ListSchemasSql =>
"SELECT DISTINCT TABLE_SCHEMA FROM INFORMATION_SCHEMA.TABLES ORDER BY TABLE_SCHEMA";
@@ -519,9 +519,9 @@ public sealed class SqlDriver
try
{
var snapshots = await _reader.ReadAsync(fullReferences, cancellationToken).ConfigureAwait(false);
ObservePollOutcome(snapshots);
return snapshots;
var result = await _reader.ReadDetailedAsync(fullReferences, cancellationToken).ConfigureAwait(false);
ObservePollOutcome(result.Snapshots, result.RowCapExceededGroups);
return result.Snapshots;
}
catch (OperationCanceledException)
{
@@ -608,7 +608,13 @@ public sealed class SqlDriver
/// cadence rather than backed off from.</para>
/// </summary>
/// <param name="snapshots">The snapshots the reader returned for one poll.</param>
internal void ObservePollOutcome(IReadOnlyList<DataValueSnapshot> snapshots)
/// <param name="answeredRefusals">
/// Groups the database answered but the driver refused (an over-cap result set, R06:S-1). Their
/// members carry <see cref="SqlStatusCodes.BadConfigurationError"/>, which alone is not answer evidence
/// (the planner mints it before any I/O), so the count is what makes them count as the database
/// answering.
/// </param>
internal void ObservePollOutcome(IReadOnlyList<DataValueSnapshot> snapshots, int answeredRefusals = 0)
{
if (snapshots.Count == 0) return;
@@ -625,6 +631,8 @@ public sealed class SqlDriver
if (IsAnswerEvidence(snapshot.StatusCode)) answered++;
}
answered += answeredRefusals;
if (unreachable > 0)
{
Degrade(
@@ -660,17 +668,12 @@ public sealed class SqlDriver
if (answered == 0) return; // authoring-class Bad only — not a statement about the database.
if (good == 0)
{
// The database answered, just not with a value: an absent row, a NULL cell, a cell that would
// not coerce. Those are data/authoring facts, so the health STATE is deliberately left exactly
// as it was — but they are still proof the transport works, and the evidence clock the
// comms-loss watchdog reads must say so.
TouchLastSuccessfulRead();
TransitionHostTo(HostState.Running);
return;
}
// Good values, OR a database that answered with refusals only — an absent row, a NULL cell, an
// uncoercible cell, an over-cap result set. A refusal is the database answering: it advances the
// evidence clock AND restores Healthy after a comms fault, exactly as a Good read does. Leaving the
// state as it was latched a recovered database Degraded behind every poll that read only refusals.
//
// Route through the Faulted guard, not a raw WriteHealth: a poll still in flight when a concurrent
// ReinitializeAsync has recorded Faulted (the engine's ~5 s teardown wait is shorter than the 15 s
// default operationTimeout, so a stale poll can still complete afterwards) must not flip Faulted back
@@ -230,15 +230,21 @@ public static class SqlGroupPlanner
values.Add(keyValue);
}
// Row cap (R06:S-1): one row per distinct key is all this plan can use, so it asks for ONE more — enough
// to see a source breaking the one-row-per-key contract, never enough to pull a history table's every
// matching row into memory on every poll.
var rowCap = values.Count;
var sql = new StringBuilder("SELECT ")
.Append(dialect.RowLimitPrefix(rowCap + 1))
.Append(QuoteList(selected, dialect))
.Append(" FROM ").Append(QuoteQualifiedName(first.Table, dialect))
.Append(" WHERE ").Append(dialect.QuoteIdentifier(keyColumn))
.Append(" IN (").AppendJoin(", ", markers).Append(')')
.Append(dialect.RowLimitSuffix(rowCap + 1))
.ToString();
return new SqlQueryPlan(
SqlTagModel.KeyValue, groupKey, sql, markers, bindNames, values, members, selected);
SqlTagModel.KeyValue, groupKey, sql, markers, bindNames, values, members, selected, rowCap);
}
/// <summary>
@@ -270,14 +276,18 @@ 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)
{
// Capped at TWO rows (R06:S-1): the selector must identify one row, and the second is the proof it
// does not. A TOP 1 alone would hide that ambiguity (see SqlPollReader.SelectWideRow); no cap at all
// materialised every matching row of an events table on every poll.
var marker = dialect.ParameterMarker(RowSelectorBaseName);
var sql = string.Concat(
"SELECT ", columns, " FROM ", table,
" WHERE ", dialect.QuoteIdentifier(first.RowSelectorColumn!), " = ", marker);
"SELECT ", dialect.RowLimitPrefix(2), columns, " FROM ", table,
" WHERE ", dialect.QuoteIdentifier(first.RowSelectorColumn!), " = ", marker,
dialect.RowLimitSuffix(2));
return new SqlQueryPlan(
SqlTagModel.WideRow, groupKey, sql,
[marker], [dialect.ParameterBindName(RowSelectorBaseName)],
[first.RowSelectorValue], members, selected);
[first.RowSelectorValue], members, selected, RowCap: 1);
}
if (!Blank(first.RowSelectorTopByTimestamp))
@@ -162,11 +162,24 @@ public sealed class SqlPollReader : IDisposable
/// <exception cref="DbException">The database could not be reached (opening a connection failed).</exception>
public async Task<IReadOnlyList<DataValueSnapshot>> ReadAsync(
IReadOnlyList<string> fullReferences, CancellationToken cancellationToken)
=> (await ReadDetailedAsync(fullReferences, cancellationToken).ConfigureAwait(false)).Snapshots;
/// <summary>
/// <see cref="ReadAsync"/> plus what the snapshots alone cannot say: how many groups the database
/// ANSWERED with more rows than the plan can use (R06:S-1). Those groups publish
/// <see cref="SqlStatusCodes.BadConfigurationError"/> — the same code an un-plannable definition gets
/// before any I/O — so the driver needs this count to treat them as the answer evidence they are.
/// </summary>
/// <param name="fullReferences">The RawPaths to read.</param>
/// <param name="cancellationToken">The caller's token; see <see cref="ReadAsync"/>.</param>
/// <returns>The snapshots and the answered-refusal count.</returns>
public async Task<SqlPollResult> ReadDetailedAsync(
IReadOnlyList<string> fullReferences, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(fullReferences);
var results = new DataValueSnapshot[fullReferences.Count];
if (results.Length == 0) return results;
if (results.Length == 0) return new SqlPollResult(results, 0);
var readAt = DateTime.UtcNow;
@@ -189,19 +202,23 @@ public sealed class SqlPollReader : IDisposable
positions.Add(i);
}
if (definitions.Count > 0)
await ReadGroupsAsync(definitions, slots, results, readAt, cancellationToken).ConfigureAwait(false);
var rowCapExceeded = definitions.Count > 0
? await ReadGroupsAsync(definitions, slots, results, readAt, cancellationToken).ConfigureAwait(false)
: 0;
// Fail-safe. Nothing above should leave a hole, but "N in, N out" is a contract the caller indexes
// blind — a null here would be a NullReferenceException in the publish fan-out, far from its cause.
for (var i = 0; i < results.Length; i++)
results[i] ??= new DataValueSnapshot(null, SqlStatusCodes.BadInternalError, null, readAt);
return results;
return new SqlPollResult(results, rowCapExceeded);
}
/// <summary>Plans the resolved definitions, runs every group concurrently, and slices the results back.</summary>
private async Task ReadGroupsAsync(
/// <summary>
/// Plans the resolved definitions, runs every group concurrently, and slices the results back.
/// Returns how many groups came back over their row cap.
/// </summary>
private async Task<int> ReadGroupsAsync(
List<SqlTagDefinition> definitions,
Dictionary<SqlTagDefinition, List<int>> slots,
DataValueSnapshot[] results,
@@ -227,7 +244,7 @@ public sealed class SqlPollReader : IDisposable
foreach (var position in positions) results[position] = snapshot;
}
return;
return 0;
}
// Groups run concurrently; _gate — not the task count — is what bounds open connections. Each group
@@ -241,8 +258,14 @@ public sealed class SqlPollReader : IDisposable
// leaves a sibling group running against a connection nobody is watching.
var outcomes = await Task.WhenAll(groups).ConfigureAwait(false);
var rowCapExceeded = 0;
for (var g = 0; g < plans.Count; g++)
{
Slice(plans[g], outcomes[g], slots, results, readAt);
if (ReferenceEquals(outcomes[g], GroupResult.RowCapExceeded)) rowCapExceeded++;
}
return rowCapExceeded;
}
/// <summary>
@@ -502,8 +525,11 @@ public sealed class SqlPollReader : IDisposable
columnIndex[selected[c]] = c;
}
// The client-side half of the row cap (R06:S-1): the statement already asks for RowCap + 1 rows, but a
// provider that cannot spell a limit must not turn that into an unbounded read either.
var limit = plan.RowCap is { } cap ? cap + 1 : int.MaxValue;
var rows = new List<object?[]>();
while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
while (rows.Count < limit && await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
{
var row = new object?[selected.Count];
for (var c = 0; c < selected.Count; c++)
@@ -511,6 +537,25 @@ public sealed class SqlPollReader : IDisposable
rows.Add(row);
}
if (plan.RowCap is { } rowCap && rows.Count > rowCap)
{
if (ShouldWarnAboutContract())
{
var first = plan.Members[0];
var selector = plan.Model == SqlTagModel.KeyValue
? string.Concat(first.KeyColumn, " IN (", rowCap.ToString(CultureInfo.InvariantCulture), " key(s))")
: string.Concat(first.RowSelectorColumn, " = ", first.RowSelectorValue);
_logger.LogWarning(
"Sql poll source '{Table}' returned more rows than the {Model} selector '{Selector}' can use " +
"(at most {Cap}) in one poll, so the selector is ambiguous — a history/events table, or a " +
"where column that is not unique. Its {Members} tag(s) publish BadConfigurationError and " +
"nothing past row {Limit} is read. Point the tag at a current-values table or a latest-row view.",
first.Table, plan.Model, selector, rowCap, plan.Members.Count, rowCap + 1);
}
return GroupResult.RowCapExceeded;
}
return GroupResult.Ok(rows, columnIndex, typeNames);
}
@@ -609,15 +654,14 @@ public sealed class SqlPollReader : IDisposable
/// emits no <c>ORDER BY</c>, so across polls the "last" row is whatever the server returned last,
/// i.e. storage order. That is why more than one row is a reported contract violation and not merely
/// a tie to break — the model's premise (design §5.3) is that the selector identifies one row.</para>
/// <para><b>Why the where-pair form is deliberately NOT given the dialect's single-row limit.</b> A
/// <c>TOP 1</c> with no <c>ORDER BY</c> is not deterministic either — it returns whichever row the
/// plan happens to produce first, so it would trade one arbitrary row for a differently arbitrary
/// one. Worse, it would destroy the only evidence this method has that the selector is ambiguous
/// (<c>Rows.Count &gt; 1</c>), converting a loud misconfiguration into a permanently silent one —
/// precisely the failure mode this class's summary calls its top risk. The <c>topByTimestamp</c>
/// form keeps its row limit because there the <c>ORDER BY</c> at least fixes <em>which</em> rows can
/// be first. The accepted cost is that an ambiguous selector materialises every matching row for one
/// poll; the warning is what gets it fixed.</para>
/// <para><b>Why the where-pair form is capped at TWO rows, not one (R06:S-1).</b> A <c>TOP 1</c> with no
/// <c>ORDER BY</c> is not deterministic — it returns whichever row the plan produces first — and it would
/// destroy the only evidence that the selector is ambiguous, converting a loud misconfiguration into a
/// permanently silent one. No limit at all materialised every matching row of an events table on every
/// poll. Two rows keeps the evidence and bounds the read: a second row makes the group publish
/// <c>BadConfigurationError</c> in <c>MaterialiseAsync</c>, so a where-pair group never reaches this
/// method with more than one row. The multi-row warning below is now reached only by a plan with no
/// cap.</para>
/// <para><b>⚠ The <c>topByTimestamp</c> answer is only defined when the ordering column is unique</b>
/// <c>ORDER BY ts DESC</c> plus a one-row limit over a column with ties returns an
/// arbitrary row among the tied ones, and — unlike the where-pair form — this method can never
@@ -807,6 +851,13 @@ public sealed class SqlPollReader : IDisposable
public static GroupResult Failed { get; } =
new(SqlStatusCodes.BadCommunicationError, NoRows, NoColumns, []);
/// <summary>
/// The database answered with more rows than the plan can use — an ambiguous selector (R06:S-1).
/// A configuration fact, not a database fault; the driver counts it as answer evidence.
/// </summary>
public static GroupResult RowCapExceeded { get; } =
new(SqlStatusCodes.BadConfigurationError, NoRows, NoColumns, []);
/// <summary>The status every member inherits when <see cref="Succeeded"/> is false.</summary>
public uint FailureStatus { get; }
@@ -835,6 +886,14 @@ public sealed class SqlPollReader : IDisposable
}
}
/// <summary>One poll's snapshots plus the answer evidence they cannot carry themselves.</summary>
/// <param name="Snapshots">One snapshot per requested reference, at the same index.</param>
/// <param name="RowCapExceededGroups">
/// Groups the database answered with more rows than their plan can use; their members carry
/// <see cref="SqlStatusCodes.BadConfigurationError"/>.
/// </param>
public sealed record SqlPollResult(IReadOnlyList<DataValueSnapshot> Snapshots, int RowCapExceededGroups);
/// <summary>
/// The OPC UA status codes the Sql driver publishes, and the quality-class predicates its tests read
/// them back with.
@@ -55,6 +55,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// The bare (unquoted) column names in <see cref="SqlText"/>'s SELECT list, in order and distinct — the
/// reader's map from result-set ordinal to source column.
/// </param>
/// <param name="RowCap">
/// The most rows this plan can USE — its distinct key count (key-value) or 1 (a where-pair wide row);
/// <see langword="null"/> when the statement already limits itself (topByTimestamp). <see cref="SqlText"/>
/// asks for <c>RowCap + 1</c>, and the reader stops there: receiving the extra row proves the selector is
/// ambiguous (a history table, a non-unique where column) and the group publishes
/// <c>BadConfigurationError</c> instead of materialising every matching row on every poll (R06:S-1).
/// </param>
public sealed record SqlQueryPlan(
SqlTagModel Model,
object GroupKey,
@@ -63,7 +70,8 @@ public sealed record SqlQueryPlan(
IReadOnlyList<string> ParameterBindNames,
IReadOnlyList<object?> Parameters,
IReadOnlyList<SqlTagDefinition> Members,
IReadOnlyList<string> SelectedColumns)
IReadOnlyList<string> SelectedColumns,
int? RowCap = null)
{
// A plan is documented as safe to cache and reuse across polls, so it must not stay aliased to the
// planner's mutable working lists — an IReadOnlyList<T> holding a List<T> is downcastable, and a