review(Driver.Sql): findings + fix the missing default-schema fallback, the rowSelector doc, and a dead package ref

Driver.Sql-001 (Medium): SqlCatalogLoader.ReadDefaultSchemaAsync documented a
fallback to the sole visible schema when the server reports no default schema and
did not implement it, so a login with no DEFAULT_SCHEMA (or a Postgres search_path
that resolves to nothing) rejected EVERY unqualified authored table at the catalog
gate — a whole-driver BadNodeIdUnknown outage against a healthy database. Fixed;
SqlCatalogDefaultSchemaTests written red first, with a control proving the
ambiguous multi-schema case still refuses to guess.

Driver.Sql-003 (Medium): docs/drivers/Sql.md spelled the WideRow row selector at
the top level, but the parser reads those keys only inside a nested rowSelector —
a tag authored from the doc is refused and reads BadNodeIdUnknown (the #552 class,
via the documentation). Also removed the documented-but-nonexistent pollIntervalMs
per-tag field.

Driver.Sql-007 (Low): dropped the unused Microsoft.Data.SqlClient PackageReference
left behind by the .Dialect extraction (#526).

Open: 002 (no IN-list chunking vs the 2100/1000 provider bind caps), 004, 005, 006,
008, 009, 010.
This commit is contained in:
Joseph Doherty
2026-08-10 06:58:08 -04:00
parent b990adae55
commit 69ad376f0d
5 changed files with 387 additions and 11 deletions
@@ -74,7 +74,7 @@ internal static class SqlCatalogLoader
"the catalog views, which is a grant problem rather than a tag-authoring one");
var defaultSchema = await ReadDefaultSchemaAsync(
connection, dialect, timeoutSeconds, cancellationToken).ConfigureAwait(false);
connection, dialect, schemas, timeoutSeconds, cancellationToken).ConfigureAwait(false);
var tablesBySchema = new Dictionary<string, IReadOnlyList<string>>(StringComparer.Ordinal);
var columnsByTable = new Dictionary<string, IReadOnlyList<string>>(StringComparer.Ordinal);
@@ -132,12 +132,23 @@ internal static class SqlCatalogLoader
/// query answers null/blank.
/// </summary>
/// <remarks>
/// A null answer is possible (a login with no default schema mapped). Falling back to the single
/// visible schema keeps the common one-schema database working; with several visible schemas there is
/// no defensible guess, so unqualified names simply will not resolve and the gate says so by name.
/// <para>A null answer is possible (a login with no default schema mapped, or a Postgres connection
/// whose <c>search_path</c> resolves to nothing). Falling back to the single visible schema keeps the
/// common one-schema database working; with several visible schemas there is no defensible guess, so
/// unqualified names simply will not resolve and the gate says so by name.</para>
/// <para><b>The fallback is load-bearing, not cosmetic.</b> Without it a blank answer resolves the
/// default schema to the empty string, the tables query runs for schema <c>''</c>, it matches nothing,
/// and <em>every unqualified tag on the driver</em> is rejected by the gate and reads
/// <c>BadNodeIdUnknown</c> — a whole-driver outage against a healthy database with every tag correctly
/// authored. Guessing among several schemas is the opposite error (a tag silently bound to the wrong
/// relation), which is why the fallback stops at exactly one candidate.</para>
/// </remarks>
private static async Task<string> ReadDefaultSchemaAsync(
DbConnection connection, ISqlDialect dialect, int timeoutSeconds, CancellationToken cancellationToken)
DbConnection connection,
ISqlDialect dialect,
IReadOnlyList<string> schemas,
int timeoutSeconds,
CancellationToken cancellationToken)
{
var command = connection.CreateCommand();
await using (command.ConfigureAwait(false))
@@ -147,7 +158,11 @@ internal static class SqlCatalogLoader
command.CommandTimeout = timeoutSeconds;
var value = await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false);
var schema = value is null or DBNull ? null : value.ToString();
return string.IsNullOrWhiteSpace(schema) ? string.Empty : schema;
if (!string.IsNullOrWhiteSpace(schema)) return schema;
// No default schema for this login: the sole visible schema is the only defensible answer, and
// with more than one there is none — the empty string then resolves nothing, by design.
return schemas.Count == 1 ? schemas[0] : string.Empty;
}
}
@@ -18,7 +18,11 @@
<ProjectReference Include="..\ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect\ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect.csproj"/>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Data.SqlClient" />
<!-- No provider package here on purpose: every type in this project speaks System.Data.Common
only (DbProviderFactory / DbConnection / DbCommand), and the concrete provider packages live
behind the .Dialect leaf referenced above. A direct Microsoft.Data.SqlClient reference sat
here unused after that extraction and read as "this project is SQL-Server-specific", which
is exactly the assumption the dialect seam exists to prevent. -->
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
</ItemGroup>
<ItemGroup>