fix(sql-browser): rank the row-selector candidate so a date-only column is a last resort (Driver.Sql.Browser-001)
PreferredTimestampColumn nominated the first column mapping to DriverDataType .DateTime, and that set includes date-only types (SQL Server / PostgreSQL 'date'). A table whose first date/time column is a 'date' committed rowSelector.topByTimestamp = <date>, making the wide-row read ORDER BY <date> DESC LIMIT 1 - which ties for every row written that day, so 'the newest row' became storage order among the ties. Time-bearing candidates now outrank date-only ones; a date-only column is still stated when it is all the table has, so such a table stays bindable. Oracle's DATE carries a time of day and is exempt - pinned by a control dialect that differs from the SQLite one only in the Provider it reports.
This commit is contained in:
Joseph Doherty
+12
-3
@@ -190,14 +190,23 @@ commit writes as `rowSelector.topByTimestamp` ("read the newest row"). Before th
|
||||
a selector-less blob that `SqlEquipmentTagParser` and the authoring gate both refuse, so no
|
||||
browse-committed Sql tag was ever readable.
|
||||
|
||||
Two properties of the choice are deliberate:
|
||||
Three properties of the choice are deliberate:
|
||||
|
||||
- **Candidacy is by declared TYPE, never by column name.** The session asks
|
||||
`ISqlDialect.MapColumnType` for `DriverDataType.DateTime` — the same metadata the browse tree already
|
||||
renders per column. A name heuristic (`*_ts`, `*time*`) would happily order a wide-row read by a
|
||||
*string* column called `timestamp`, binding the tag to whichever row sorts last lexically.
|
||||
- **The tie-break is first in catalog order** (every dialect's column query orders by ordinal
|
||||
position), so the pick is stable across calls, connections and servers. On a
|
||||
- **Candidates are RANKED: a time-bearing type beats a date-only one.** `MapColumnType` reports
|
||||
`DriverDataType.DateTime` for SQL Server's and PostgreSQL's `date` too, and ordering a single-row read
|
||||
by a date-only column ties for *every row written on the same day* — "the newest row" quietly becomes
|
||||
storage order among the ties, and the reader cannot warn about it because the server has already
|
||||
discarded the others. So a `datetime2`/`timestamptz`/… column wins over a `date` one wherever both
|
||||
exist, regardless of declaration order. A date-only column is still stated when it is **all** the table
|
||||
has, so such a table stays bindable by browse rather than becoming un-committable.
|
||||
**Oracle's `DATE` is exempt** — it carries a time-of-day component (which is why `OracleDialect` maps
|
||||
it faithfully rather than as a widening), so it is not demoted there.
|
||||
- **The tie-break within a rank is first in catalog order** (every dialect's column query orders by
|
||||
ordinal position), so the pick is stable across calls, connections and servers. On a
|
||||
`CreatedUtc`/`ModifiedUtc` pair it takes the earlier-declared one. It is a **visible, editable
|
||||
default**: the typed editor shows the selector and the operator can change it — to the other
|
||||
timestamp, or to a `whereColumn`/`whereValue` pair entirely.
|
||||
|
||||
@@ -63,6 +63,14 @@ internal sealed class SqlBrowseSession : IBrowseSession
|
||||
/// </summary>
|
||||
private const string ReadOnlySecurityClass = "ViewOnly";
|
||||
|
||||
/// <summary>
|
||||
/// The one SQL type family that maps to <see cref="DriverDataType.DateTime"/> while carrying <b>no
|
||||
/// time of day</b> — SQL Server's and PostgreSQL's <c>date</c>. Ranked below the time-bearing
|
||||
/// families by <see cref="PreferredTimestampColumn"/>; see <see cref="IsDateOnly"/> for the Oracle
|
||||
/// exception.
|
||||
/// </summary>
|
||||
private const string DateOnlyTypeFamily = "date";
|
||||
|
||||
private readonly DbConnection _connection;
|
||||
private readonly ISqlDialect _dialect;
|
||||
|
||||
@@ -274,12 +282,23 @@ internal sealed class SqlBrowseSession : IBrowseSession
|
||||
/// 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>Ranked, because "maps to DateTime" includes DATE-ONLY types.</b>
|
||||
/// <see cref="ISqlDialect.MapColumnType"/> reports <see cref="DriverDataType.DateTime"/> for SQL
|
||||
/// Server's and PostgreSQL's <c>date</c> as well — and a wide-row read ordered by a date-only column
|
||||
/// is <c>ORDER BY <date> DESC</c> limited to one row, which <em>ties for every row written on
|
||||
/// the same day</em>: "the newest row" silently becomes storage order among the ties, and the reader
|
||||
/// cannot warn about it because the server has already discarded the others. So a time-bearing
|
||||
/// candidate always outranks a date-only one, wherever the two are declared.
|
||||
/// <b>Falling back to a date-only column is deliberate</b> rather than dropping those types from
|
||||
/// candidacy: a table whose only date/time column is a <c>date</c> stays bindable by browse (with a
|
||||
/// default the operator can see and change) instead of becoming un-committable.</para>
|
||||
/// <para><b>Tie-break within a rank: 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 candidate 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.
|
||||
@@ -289,13 +308,42 @@ internal sealed class SqlBrowseSession : IBrowseSession
|
||||
/// </remarks>
|
||||
private string? PreferredTimestampColumn(IReadOnlyList<(string Name, string DataType)> columns)
|
||||
{
|
||||
// One pass, remembering the best date-only candidate seen so far: the first time-bearing column wins
|
||||
// outright, and the date-only one is only used if the walk ends without finding one.
|
||||
string? dateOnlyFallback = null;
|
||||
|
||||
foreach (var column in columns)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(column.Name)) continue;
|
||||
if (_dialect.MapColumnType(column.DataType) == DriverDataType.DateTime) return column.Name;
|
||||
if (_dialect.MapColumnType(column.DataType) != DriverDataType.DateTime) continue;
|
||||
|
||||
if (!IsDateOnly(column.DataType)) return column.Name;
|
||||
dateOnlyFallback ??= column.Name;
|
||||
}
|
||||
|
||||
return null;
|
||||
return dateOnlyFallback;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Whether a column's declared type family carries a date but no time of day — the case that makes an
|
||||
/// <c>ORDER BY … DESC</c> single-row read tie for a whole day's rows.
|
||||
/// </summary>
|
||||
/// <param name="sqlDataType">The bare SQL type family name, as the catalog reported it.</param>
|
||||
/// <returns><see langword="true"/> when the family is date-only on this dialect's backend.</returns>
|
||||
/// <remarks>
|
||||
/// <b>Provider-qualified, because the spelling is not the meaning.</b> <c>date</c> is date-only on SQL
|
||||
/// Server and PostgreSQL, but <b>Oracle's <c>DATE</c> carries a time-of-day component</b> — which is
|
||||
/// why <c>OracleDialect.MapColumnType</c> calls <see cref="DriverDataType.DateTime"/> the faithful
|
||||
/// mapping there rather than a widening. Demoting it on Oracle would change the pick on the backend
|
||||
/// where <c>DATE</c> is the ordinary timestamp type, for no gain.
|
||||
/// <para>The check is a browse-time <em>preference</em> and belongs here rather than on
|
||||
/// <see cref="ISqlDialect"/>: nothing on the poll path cares, and the type map is deliberately about
|
||||
/// what a value IS, not about what it is a good idea to sort by.</para>
|
||||
/// </remarks>
|
||||
private bool IsDateOnly(string sqlDataType)
|
||||
{
|
||||
if (_dialect.Provider == SqlProvider.Oracle) return false;
|
||||
return string.Equals(sqlDataType.Trim(), DateOnlyTypeFamily, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
+64
@@ -0,0 +1,64 @@
|
||||
using System.Data.Common;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
|
||||
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// <see cref="SqliteDialect"/> in every respect except that it reports <see cref="SqlProvider.Oracle"/>.
|
||||
/// It exists for exactly one assertion: <b>Oracle's <c>DATE</c> carries a time-of-day component</b>
|
||||
/// (<c>OracleDialect.MapColumnType</c> says so in as many words), so it is NOT the date-only type the
|
||||
/// row-selector ranking demotes on the other dialects, and an Oracle table whose <c>DATE</c> column is
|
||||
/// declared before its <c>TIMESTAMP</c> must still pick the <c>DATE</c> — the ordinary
|
||||
/// first-in-catalog-order tie-break.
|
||||
/// <para>Without this control, implementing the ranking provider-blind ("any type family spelled
|
||||
/// <c>date</c> is date-only") passes every other test in the suite while silently changing the pick on
|
||||
/// the one backend where the spelling means something else.</para>
|
||||
/// </summary>
|
||||
public sealed class OracleFlavouredSqliteDialect : ISqlDialect
|
||||
{
|
||||
private readonly SqliteDialect _inner = new();
|
||||
|
||||
/// <summary>The whole point of the type: an Oracle-shaped provider over the SQLite catalog.</summary>
|
||||
public SqlProvider Provider => SqlProvider.Oracle;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public DbProviderFactory Factory => _inner.Factory;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string LivenessSql => _inner.LivenessSql;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string SingleRowLimitPrefix => _inner.SingleRowLimitPrefix;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string SingleRowLimitSuffix => _inner.SingleRowLimitSuffix;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string ListSchemasSql => _inner.ListSchemasSql;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string DefaultSchemaSql => _inner.DefaultSchemaSql;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string ListTablesSql => _inner.ListTablesSql;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string ListColumnsSql => _inner.ListColumnsSql;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string QuoteIdentifier(string ident) => _inner.QuoteIdentifier(ident);
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string ParameterMarker(string baseName) => _inner.ParameterMarker(baseName);
|
||||
|
||||
/// <inheritdoc/>
|
||||
public string ParameterBindName(string baseName) => _inner.ParameterBindName(baseName);
|
||||
|
||||
/// <inheritdoc/>
|
||||
public void ConfigureCommand(DbCommand command) => _inner.ConfigureCommand(command);
|
||||
|
||||
/// <inheritdoc/>
|
||||
public DriverDataType MapColumnType(string sqlDataType) => _inner.MapColumnType(sqlDataType);
|
||||
}
|
||||
@@ -458,6 +458,83 @@ public sealed class SqlBrowseSessionTests
|
||||
.ShouldBeGreaterThan(0, "the expected pick must NOT also be the alphabetically first one");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A <b>date-only</b> column is a candidate of last resort, not the first pick. <c>date</c> maps to
|
||||
/// <see cref="DriverDataType.DateTime"/> on every dialect, so a candidacy rule that only asked the
|
||||
/// mapped type would commit <c>ORDER BY <date> DESC LIMIT 1</c> — which ties for every row
|
||||
/// written on the same day, making "the newest row" storage order among the ties. The reader cannot
|
||||
/// warn about it either: the server has already discarded the other rows.
|
||||
/// <para>The fixture's table declares the date-only column FIRST, so this fails against the plain
|
||||
/// first-in-catalog-order rule rather than passing by luck.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task AttributesAsync_prefersATimeBearingColumn_overAnEarlierDateOnlyOne()
|
||||
{
|
||||
await using var db = await SqliteBrowseFixture.CreateAsync();
|
||||
await using var session = new SqlBrowseSession(db.Connection, new SqliteDialect());
|
||||
|
||||
var nodeId = SqlBrowseNodeId.ForColumn(
|
||||
SqliteDialect.MainSchema,
|
||||
SqliteBrowseFixture.DateOnlyFirstTable,
|
||||
SqliteBrowseFixture.DateOnlyFirstValueColumn);
|
||||
|
||||
var fields = (await session.AttributesAsync(nodeId, CancellationToken.None))
|
||||
.ShouldHaveSingleItem().AddressFields.ShouldNotBeNull();
|
||||
|
||||
fields[SqlBrowseAddressFields.TopByTimestamp].ShouldBe(
|
||||
SqliteBrowseFixture.DateOnlyFirstTimeColumn,
|
||||
"a time-bearing column outranks a date-only one even when the date-only one is declared first");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The fallback half of the ranking: when date-only is ALL the table has, it is still stated. Dropping
|
||||
/// date-only types from candidacy outright would instead make such a table unbindable by browse, which
|
||||
/// is a bigger behaviour change than the defect being fixed.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task AttributesAsync_stillStatesADateOnlyColumn_whenTheTableHasNothingBetter()
|
||||
{
|
||||
await using var db = await SqliteBrowseFixture.CreateAsync();
|
||||
await using var session = new SqlBrowseSession(db.Connection, new SqliteDialect());
|
||||
|
||||
var nodeId = SqlBrowseNodeId.ForColumn(
|
||||
SqliteDialect.MainSchema,
|
||||
SqliteBrowseFixture.DateOnlySoleTable,
|
||||
SqliteBrowseFixture.DateOnlySoleValueColumn);
|
||||
|
||||
var fields = (await session.AttributesAsync(nodeId, CancellationToken.None))
|
||||
.ShouldHaveSingleItem().AddressFields.ShouldNotBeNull();
|
||||
|
||||
fields[SqlBrowseAddressFields.TopByTimestamp].ShouldBe(SqliteBrowseFixture.DateOnlySoleColumn);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The falsification control for the ranking: <b>Oracle's <c>DATE</c> carries a time-of-day
|
||||
/// component</b>, so it is not date-only there and must not be demoted. Same catalog, same columns,
|
||||
/// only the dialect's <see cref="ISqlDialect.Provider"/> differs — and the pick reverts to the
|
||||
/// ordinary first-in-catalog-order one.
|
||||
/// <para>A ranking implemented provider-blind ("any family spelled <c>date</c>") passes every other
|
||||
/// test in this file and fails only here.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task AttributesAsync_doesNotDemoteOraclesDATE_whichCarriesATimeOfDay()
|
||||
{
|
||||
await using var db = await SqliteBrowseFixture.CreateAsync();
|
||||
await using var session = new SqlBrowseSession(db.Connection, new OracleFlavouredSqliteDialect());
|
||||
|
||||
var nodeId = SqlBrowseNodeId.ForColumn(
|
||||
SqliteDialect.MainSchema,
|
||||
SqliteBrowseFixture.DateOnlyFirstTable,
|
||||
SqliteBrowseFixture.DateOnlyFirstValueColumn);
|
||||
|
||||
var fields = (await session.AttributesAsync(nodeId, CancellationToken.None))
|
||||
.ShouldHaveSingleItem().AddressFields.ShouldNotBeNull();
|
||||
|
||||
fields[SqlBrowseAddressFields.TopByTimestamp].ShouldBe(
|
||||
SqliteBrowseFixture.DateOnlyFirstColumn,
|
||||
"Oracle's DATE is time-bearing, so the first-in-catalog-order tie-break applies unchanged");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A table with no date/time column states NO candidate — the key is absent, never blank — so the
|
||||
/// commit can tell "this table cannot be bound by browse" from "the session said nothing".
|
||||
|
||||
@@ -96,6 +96,40 @@ public sealed class SqliteBrowseFixture : IAsyncDisposable
|
||||
/// <summary>A <see cref="MultiTimeTable"/> value column, sitting BETWEEN the two timestamps.</summary>
|
||||
public const string MultiTimeValueColumn = "level";
|
||||
|
||||
/// <summary>
|
||||
/// A table whose FIRST date/time-typed column is <b>date-only</b>, with a genuinely time-bearing one
|
||||
/// declared after it. A date-only column maps to
|
||||
/// <see cref="ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverDataType.DateTime"/> like any other, so a
|
||||
/// candidacy rule that only asked the mapped type would order the wide-row read by
|
||||
/// <see cref="DateOnlyFirstColumn"/> — one row per DAY, ties broken by storage order.
|
||||
/// </summary>
|
||||
public const string DateOnlyFirstTable = "DailyTotals";
|
||||
|
||||
/// <summary>The date-only column of <see cref="DateOnlyFirstTable"/>, declared first.</summary>
|
||||
public const string DateOnlyFirstColumn = "shift_day";
|
||||
|
||||
/// <summary>A <see cref="DateOnlyFirstTable"/> value column, sitting between the two candidates.</summary>
|
||||
public const string DateOnlyFirstValueColumn = "units";
|
||||
|
||||
/// <summary>
|
||||
/// The time-bearing column of <see cref="DateOnlyFirstTable"/>, declared LAST — the expected pick,
|
||||
/// even though the date-only one comes first in catalog order.
|
||||
/// </summary>
|
||||
public const string DateOnlyFirstTimeColumn = "logged_at";
|
||||
|
||||
/// <summary>
|
||||
/// A table whose ONLY date/time-typed column is date-only — the fallback control. Dropping date-only
|
||||
/// types from candidacy outright would make this table unbindable by browse; ranking keeps it bindable
|
||||
/// while still preferring a better column wherever one exists.
|
||||
/// </summary>
|
||||
public const string DateOnlySoleTable = "DayBook";
|
||||
|
||||
/// <summary>The sole (date-only) candidate of <see cref="DateOnlySoleTable"/>.</summary>
|
||||
public const string DateOnlySoleColumn = "book_day";
|
||||
|
||||
/// <summary>A <see cref="DateOnlySoleTable"/> value column.</summary>
|
||||
public const string DateOnlySoleValueColumn = "opening_balance";
|
||||
|
||||
private readonly string _databasePath;
|
||||
|
||||
private SqliteBrowseFixture(string databasePath, string connectionString, SqliteConnection connection)
|
||||
@@ -210,6 +244,17 @@ public sealed class SqliteBrowseFixture : IAsyncDisposable
|
||||
"{MultiTimeValueColumn}" REAL NULL,
|
||||
"{MultiTimeSecondColumn}" TIMESTAMP NULL
|
||||
);
|
||||
|
||||
CREATE TABLE "{DateOnlyFirstTable}" (
|
||||
"{DateOnlyFirstColumn}" DATE NOT NULL,
|
||||
"{DateOnlyFirstValueColumn}" REAL NULL,
|
||||
"{DateOnlyFirstTimeColumn}" DATETIME NOT NULL
|
||||
);
|
||||
|
||||
CREATE TABLE "{DateOnlySoleTable}" (
|
||||
"{DateOnlySoleColumn}" DATE NOT NULL,
|
||||
"{DateOnlySoleValueColumn}" REAL NULL
|
||||
);
|
||||
""").ConfigureAwait(false);
|
||||
|
||||
// Separate statements: these names must be quoted by the seed itself, so keep them off the
|
||||
|
||||
Reference in New Issue
Block a user