refactor(sql): promote the single-row limit onto ISqlDialect

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-24 13:57:25 -04:00
parent 984cc875e8
commit 3d7e1226d2
4 changed files with 72 additions and 17 deletions
@@ -6,9 +6,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// <summary> /// <summary>
/// The provider seam (design §2.2). ADO.NET's <see cref="System.Data.Common"/> base types abstract /// The provider seam (design §2.2). ADO.NET's <see cref="System.Data.Common"/> base types abstract
/// connections, commands, parameters and readers; they do <b>not</b> abstract the two things that differ /// connections, commands, parameters and readers; they do <b>not</b> abstract the three things that differ
/// per backend — <b>identifier quoting</b> and the <b>metadata-catalog SQL</b>. Those live here, so no /// per backend — <b>identifier quoting</b>, the <b>metadata-catalog SQL</b>, and <b>row-limit syntax</b>.
/// dialect assumption leaks into the reader, the poll planner, or the schema browser. /// Those live here, so no dialect assumption leaks into the reader, the poll planner, or the schema browser.
/// <para><b>This interface is the driver's SQL-injection boundary.</b> Every runtime <em>value</em> is /// <para><b>This interface is the driver's SQL-injection boundary.</b> Every runtime <em>value</em> is
/// bound as a <see cref="DbParameter"/> and never reaches a command text. Identifiers cannot be /// bound as a <see cref="DbParameter"/> and never reaches a command text. Identifiers cannot be
/// parameterized in SQL, so the few that must appear as text are sourced only from catalog-validated /// parameterized in SQL, so the few that must appear as text are sourced only from catalog-validated
@@ -47,6 +47,41 @@ public interface ISqlDialect
/// <summary>Cheapest statement that proves the connection is alive (<c>SELECT 1</c>; Oracle needs <c>FROM DUAL</c>).</summary> /// <summary>Cheapest statement that proves the connection is alive (<c>SELECT 1</c>; Oracle needs <c>FROM DUAL</c>).</summary>
string LivenessSql { get; } string LivenessSql { get; }
/// <summary>
/// The fragment that goes <b>immediately after <c>SELECT</c></b> to limit a statement to one row —
/// T-SQL's <c>"TOP 1 "</c>. Empty for a dialect that spells the limit at the end of the statement (see
/// <see cref="SingleRowLimitSuffix"/>).
/// <para><b>Include the trailing space</b> when non-empty: the planner concatenates
/// <c>"SELECT " + SingleRowLimitPrefix + columns</c> with no separator of its own, so an empty fragment
/// must leave the statement byte-identical to an unlimited one.</para>
/// </summary>
/// <remarks>
/// <para><b>Why a prefix/suffix pair rather than one <c>ApplyRowLimit(sql, n)</c> method.</b> The two
/// spellings sit at opposite ends of the statement — SQL Server writes
/// <c>SELECT TOP 1 … ORDER BY … DESC</c>, while SQLite/Postgres/MySQL write
/// <c>SELECT … ORDER BY … DESC LIMIT 1</c>. A single method taking the SELECT list could only serve the
/// prefix position; a single method taking the whole statement would move statement assembly out of the
/// planner and into every dialect. Two fragments keep the planner the sole author of statement shape
/// and let each dialect fill in the end it uses.</para>
/// <para><b>Why "single row" rather than a row count.</b> The only limit v1 emits is the wide-row
/// <c>topByTimestamp</c> selector's newest-row pick. Modelling an arbitrary <c>n</c> would be untested
/// surface, and <c>FETCH FIRST</c>/<c>ROWNUM</c> dialects need more than a substituted number anyway.
/// Widen this to a method when a feature actually needs <c>n &gt; 1</c>.</para>
/// </remarks>
string SingleRowLimitPrefix { get; }
/// <summary>
/// The fragment appended to the <b>very end</b> of a statement to limit it to one row —
/// <c>" LIMIT 1"</c> for SQLite/Postgres/MySQL. Empty for a dialect that limits after <c>SELECT</c>
/// (see <see cref="SingleRowLimitPrefix"/>).
/// <para><b>Include the leading space</b> when non-empty, for the same reason the prefix carries a
/// trailing one: the planner appends it directly to the finished statement.</para>
/// <para>Exactly one of the two fragments is non-empty in every dialect modelled so far, but the planner
/// emits <b>both</b> unconditionally — a dialect needing both ends (or neither) is expressible without
/// touching the call site.</para>
/// </summary>
string SingleRowLimitSuffix { get; }
/// <summary>Catalog query listing schemas — the browser's <c>RootAsync</c> level. Takes no parameters.</summary> /// <summary>Catalog query listing schemas — the browser's <c>RootAsync</c> level. Takes no parameters.</summary>
string ListSchemasSql { get; } string ListSchemasSql { get; }
@@ -41,8 +41,7 @@ public static class SqlGroupPlanner
/// up front, so reaching one here means a caller built a definition by hand with a broken invariant. /// up front, so reaching one here means a caller built a definition by hand with a broken invariant.
/// </exception> /// </exception>
/// <exception cref="NotSupportedException"> /// <exception cref="NotSupportedException">
/// A tag carries <see cref="SqlTagModel.Query"/> (design §5.4, deferred to P3), or a /// A tag carries <see cref="SqlTagModel.Query"/> (design §5.4, deferred to P3).
/// <c>topByTimestamp</c> selector is planned for a provider whose row-limit syntax is not yet modelled.
/// <b>Deliberately loud rather than skipped:</b> silently dropping a tag would leave an authored node /// <b>Deliberately loud rather than skipped:</b> silently dropping a tag would leave an authored node
/// permanently unfed with nothing in the log, and the parser makes this branch unreachable in practice. /// permanently unfed with nothing in the log, and the parser makes this branch unreachable in practice.
/// </exception> /// </exception>
@@ -157,7 +156,10 @@ public static class SqlGroupPlanner
/// <summary> /// <summary>
/// Emits <c>SELECT &lt;cols&gt; FROM &lt;table&gt; WHERE &lt;whereColumn&gt; = @w</c>, or — for a /// Emits <c>SELECT &lt;cols&gt; FROM &lt;table&gt; WHERE &lt;whereColumn&gt; = @w</c>, or — for a
/// <c>topByTimestamp</c> selector — <c>SELECT TOP 1 &lt;cols&gt; FROM &lt;table&gt; ORDER BY &lt;ts&gt; DESC</c>. /// <c>topByTimestamp</c> selector — the single-row form
/// <c>SELECT &lt;limit&gt; &lt;cols&gt; FROM &lt;table&gt; ORDER BY &lt;ts&gt; DESC &lt;limit&gt;</c>, whose
/// row limit comes from <see cref="ISqlDialect.SingleRowLimitPrefix"/> /
/// <see cref="ISqlDialect.SingleRowLimitSuffix"/> (T-SQL fills the prefix: <c>SELECT TOP 1 …</c>).
/// <para>The SELECT list is each member's <c>columnName</c> (distinct, in first-appearance order), /// <para>The SELECT list is each member's <c>columnName</c> (distinct, in first-appearance order),
/// followed by each distinct member <c>timestampColumn</c> — so members of one row may carry different /// followed by each distinct member <c>timestampColumn</c> — so members of one row may carry different
/// timestamp columns without splitting the group. The where-column itself is <b>not</b> selected: every /// timestamp columns without splitting the group. The where-column itself is <b>not</b> selected: every
@@ -191,18 +193,13 @@ public static class SqlGroupPlanner
if (!Blank(first.RowSelectorTopByTimestamp)) if (!Blank(first.RowSelectorTopByTimestamp))
{ {
// "TOP 1" is T-SQL. Other providers spell the row limit differently (LIMIT 1, FETCH FIRST, // The row limit is dialect syntax and sits at opposite ends of the statement per provider
// ROWNUM), and ISqlDialect does not model it yet because v1 constructs SqlServer only — so refuse // (T-SQL "SELECT TOP 1 …", SQLite/Postgres/MySQL "… LIMIT 1"), so both ends are emitted
// loudly rather than emit a statement that would not parse. Promote a row-limit member onto // unconditionally and the dialect decides which one it fills. See ISqlDialect.SingleRowLimitPrefix.
// ISqlDialect when P2 adds Postgres/ODBC.
if (dialect.Provider != SqlProvider.SqlServer)
throw new NotSupportedException(
$"The wide-row 'topByTimestamp' row selector needs a provider-specific row limit; " +
$"{dialect.Provider} is not modelled yet (v1 constructs SqlServer only).");
var sql = string.Concat( var sql = string.Concat(
"SELECT TOP 1 ", columns, " FROM ", table, "SELECT ", dialect.SingleRowLimitPrefix, columns, " FROM ", table,
" ORDER BY ", dialect.QuoteIdentifier(first.RowSelectorTopByTimestamp!), " DESC"); " 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);
} }
@@ -29,6 +29,16 @@ public sealed class SqlServerDialect : ISqlDialect
/// <inheritdoc/> /// <inheritdoc/>
public string LivenessSql => "SELECT 1"; public string LivenessSql => "SELECT 1";
/// <summary>
/// T-SQL limits after the <c>SELECT</c> keyword, so the whole limit lives in the prefix —
/// <c>"TOP 1 "</c>, trailing space included so <c>SELECT TOP 1 [col]</c> composes with no extra
/// separator at the call site.
/// </summary>
public string SingleRowLimitPrefix => "TOP 1 ";
/// <summary>Empty — T-SQL has no trailing row-limit clause (<c>OFFSET/FETCH</c> is a paging construct, not this).</summary>
public string SingleRowLimitSuffix => string.Empty;
/// <inheritdoc/> /// <inheritdoc/>
public string ListSchemasSql => public string ListSchemasSql =>
"SELECT DISTINCT TABLE_SCHEMA FROM INFORMATION_SCHEMA.TABLES ORDER BY TABLE_SCHEMA"; "SELECT DISTINCT TABLE_SCHEMA FROM INFORMATION_SCHEMA.TABLES ORDER BY TABLE_SCHEMA";
@@ -48,6 +48,19 @@ public class SqlServerDialectTests
d.ListColumnsSql.ShouldContain("@table"); d.ListColumnsSql.ShouldContain("@table");
} }
[Fact]
public void SingleRowLimit_isTSqlsTop1_inThePrefixPosition_withItsOwnTrailingSpace()
{
var d = new SqlServerDialect();
// The planner concatenates "SELECT " + prefix + columns with no separator of its own, so the
// trailing space belongs to the fragment. T-SQL has no end-of-statement limit clause.
d.SingleRowLimitPrefix.ShouldBe("TOP 1 ");
d.SingleRowLimitSuffix.ShouldBe("");
string.Concat("SELECT ", d.SingleRowLimitPrefix, "[a]", d.SingleRowLimitSuffix)
.ShouldBe("SELECT TOP 1 [a]");
}
// ---- extra guards on the injection boundary (beyond the plan's golden set) ---- // ---- extra guards on the injection boundary (beyond the plan's golden set) ----
[Fact] [Fact]