fix(sql): browse-commit states a topByTimestamp row selector (#552)

`RawBrowseCommitMapper.BuildSqlTagConfig` emitted a WideRow blob carrying only
table + columnName. A wide-row tag must also say WHICH row it reads, so both
`SqlEquipmentTagParser` (the runtime authority) and `SqlTagConfigModel.Validate`
(the authoring gate) refused it — a browse-committed Sql tag was never readable,
first silently as BadNodeIdUnknown and, since #551 put the gate on the import,
loudly at commit.

`SqlBrowseSession` now states a fourth address field: the table's preferred
date/time-typed column, computed from the columns its attributes call ALREADY
read (no extra catalog round-trip). Candidacy is decided by
`ISqlDialect.MapColumnType` reporting `DriverDataType.DateTime` — never by column
name, which would order a wide-row read by a string column that merely looks like
a timestamp. Multiple candidates tie-break on FIRST IN CATALOG ORDER (every
dialect's column SQL orders by ordinal position), so the pick is stable across
calls and servers; it is a visible, editable default the typed editor round-trips.

No candidate (a table with no date/time column) is an operator-actionable refusal,
not an invented selector: the only remaining selector is a whereColumn/whereValue
pair whose value is data, not schema, so `DescribeUncommittableLeaf` refuses that
leaf by name, names the table, and points at the tag editor.

The session and the mapper now agree via `SqlBrowseAddressFields` constants rather
than two independently spelled string literals — a disagreement there is silent,
and is exactly the shape this defect had.

`RawTagConfigGateTests`' Sql leg flips from "records the gap" to "browse-commit
output passes the gate", with the no-candidate case pinned as its own test.
This commit is contained in:
Joseph Doherty
2026-08-10 01:20:12 -04:00
parent f896e0631c
commit 7498d39e5b
8 changed files with 650 additions and 28 deletions
@@ -1,6 +1,8 @@
using System.Data;
using System.Data.Common;
using ZB.MOM.WW.OtOpcUa.Commons.Browsing;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser;
@@ -149,12 +151,34 @@ internal sealed class SqlBrowseSession : IBrowseSession
var columns = await ReadColumnsAsync(
reference.Schema, reference.Table!, cancellationToken).ConfigureAwait(false);
// Computed from the columns ALREADY READ for this call — the catalog is not queried a second time.
var timestampColumn = PreferredTimestampColumn(columns);
// Ordinal: a column NodeId is only ever minted from this same catalog output, so its case is the
// catalog's own. A case-insensitive match would pick the wrong column on a case-sensitive collation
// that legitimately carries both "Value" and "value".
foreach (var column in columns)
{
if (!string.Equals(column.Name, reference.Column, StringComparison.Ordinal)) continue;
// A column NAME alone cannot address a Sql tag — the table has to travel with it, or the
// browse-commit has nothing to build a SqlTagConfigModel from and falls through to the
// generic {"address": ...} blob the typed editor cannot read (deferment.md G-6). The
// schema is carried separately so the commit mapper can qualify the table itself rather
// than parsing a joined string back apart.
var addressFields = new Dictionary<string, string>(StringComparer.Ordinal)
{
[SqlBrowseAddressFields.Schema] = reference.Schema,
[SqlBrowseAddressFields.Table] = reference.Table!,
[SqlBrowseAddressFields.ColumnName] = column.Name,
};
// The third part of a wide-row binding: WHICH row. Stated only when the catalog actually names
// a date/time column; the key is absent otherwise, never blank, so the mapper can tell "this
// table cannot be bound by browse" from "an older session said nothing".
if (timestampColumn is not null)
addressFields[SqlBrowseAddressFields.TopByTimestamp] = timestampColumn;
return
[
new AttributeInfo(
@@ -163,17 +187,7 @@ internal sealed class SqlBrowseSession : IBrowseSession
IsArray: false,
SecurityClass: ReadOnlySecurityClass,
IsAlarm: false,
// A column NAME alone cannot address a Sql tag — the table has to travel with it, or the
// browse-commit has nothing to build a SqlTagConfigModel from and falls through to the
// generic {"address": ...} blob the typed editor cannot read (deferment.md G-6). The
// schema is carried separately so the commit mapper can qualify the table itself rather
// than parsing a joined string back apart.
AddressFields: new Dictionary<string, string>(StringComparer.Ordinal)
{
["schema"] = reference.Schema,
["table"] = reference.Table!,
["columnName"] = column.Name,
}),
AddressFields: addressFields),
];
}
@@ -245,6 +259,45 @@ internal sealed class SqlBrowseSession : IBrowseSession
return nodes;
}
/// <summary>
/// Picks the table's preferred <c>topByTimestamp</c> column — the one a browse-committed wide-row tag
/// orders by to read the newest row (Gitea #552) — or <see langword="null"/> when the table carries no
/// date/time column at all.
/// </summary>
/// <param name="columns">The table's columns, in the catalog order the dialect's column SQL returns.</param>
/// <returns>The chosen column name, or <see langword="null"/> when there is no candidate.</returns>
/// <remarks>
/// <para><b>Candidacy is decided by the column's declared TYPE, never by its name.</b> A name test
/// (<c>*_ts</c>, <c>*time*</c>, …) is a guess that reads as knowledge: it is locale-dependent, it
/// silently misses <c>Recorded</c>, and — the part that actually breaks data — it would happily order
/// a wide-row read by a <em>string</em> column called <c>timestamp</c>, binding the tag to whatever
/// row sorts last lexically. So the only input is
/// <see cref="ISqlDialect.MapColumnType"/> reporting <see cref="DriverDataType.DateTime"/>, which is
/// the same catalog metadata the browse tree already renders per column, on every dialect.</para>
/// <para><b>Tie-break: FIRST in catalog order.</b> Every dialect's <see cref="ISqlDialect.ListColumnsSql"/>
/// orders by ordinal position (<c>ORDINAL_POSITION</c> / <c>column_id</c>), so "first" means the
/// earliest-declared date/time column and is stable across calls, connections and servers — an
/// unordered "any candidate" would let two operators browsing the same table commit two differently
/// bound tags. On the usual multi-candidate shape (a <c>CreatedUtc</c>/<c>ModifiedUtc</c> pair) it
/// picks the earlier-declared one, which is a preference and not a fact about the data.</para>
/// <para><b>It is a visible, editable DEFAULT, not a hidden binding.</b> The commit writes it into the
/// tag's <c>rowSelector.topByTimestamp</c>, where the typed Sql editor shows it and the operator can
/// change it — to the other timestamp, or to a <c>whereColumn</c>/<c>whereValue</c> pair entirely.
/// Nothing about the choice is recoverable only from the browse session.</para>
/// <para>The browsed column itself is deliberately NOT excluded: picking a table's timestamp column as
/// a tag reads the newest row's timestamp, which is a legitimate (and self-consistent) tag.</para>
/// </remarks>
private string? PreferredTimestampColumn(IReadOnlyList<(string Name, string DataType)> columns)
{
foreach (var column in columns)
{
if (string.IsNullOrWhiteSpace(column.Name)) continue;
if (_dialect.MapColumnType(column.DataType) == DriverDataType.DateTime) return column.Name;
}
return null;
}
/// <summary>
/// Reads one table's columns. A table that no longer exists (or never did) yields an empty list rather
/// than an error: the catalog answering "no rows" is indistinguishable from a genuinely column-less
@@ -0,0 +1,37 @@
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
/// <summary>
/// The keys <c>SqlBrowseSession</c> states in a browsed column's <c>AttributeInfo.AddressFields</c>, and
/// that <c>RawBrowseCommitMapper</c> reads back when it builds the committed tag's <c>TagConfig</c>.
/// <para><b>Why a shared constant rather than a literal at each end.</b> A Sql tag's binding is a tuple
/// (table + column + which row), so — exactly as for MQTT's Sparkplug tuple and
/// <c>MqttTagConfigKeys</c> — the producing session states the parts and the mapper must never
/// re-infer them. Two independently spelled string literals agree by coincidence, and a disagreement is
/// silent: the mapper simply sees no field, and emits a config missing whatever the session was trying
/// to say. That is precisely how Gitea #552 shipped — a wide-row blob with no <c>rowSelector</c>, which
/// both <see cref="SqlEquipmentTagParser"/> and the authoring gate refuse.</para>
/// <para>These are the <em>browse channel's</em> names, not the tag blob's schema. Three of them happen
/// to match a <c>TagConfig</c> key because the mapper writes them straight through;
/// <see cref="TopByTimestamp"/> matches the key <em>nested inside</em> <c>rowSelector</c> — deliberately
/// not spelled <c>timestampColumn</c>, which is the blob's unrelated top-level source-timestamp
/// field.</para>
/// </summary>
public static class SqlBrowseAddressFields
{
/// <summary>The catalog schema owning the browsed column's table (e.g. <c>dbo</c>, <c>sales</c>).</summary>
public const string Schema = "schema";
/// <summary>The unqualified table (or view) name the browsed column belongs to.</summary>
public const string Table = "table";
/// <summary>The browsed column's own name — the wide-row tag's value column.</summary>
public const string ColumnName = "columnName";
/// <summary>
/// The table's preferred date/time-typed column, when it has one: the commit writes it as the
/// wide-row <c>rowSelector.topByTimestamp</c>, i.e. "read the newest row". Absent when the table
/// carries no date/time column at all, which is not a shape a browse can bind — see
/// <c>RawBrowseCommitMapper.DescribeUncommittableLeaf</c>.
/// </summary>
public const string TopByTimestamp = "topByTimestamp";
}