diff --git a/docs/drivers/Sql.md b/docs/drivers/Sql.md index 38163f50..575a5128 100644 --- a/docs/drivers/Sql.md +++ b/docs/drivers/Sql.md @@ -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. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs index 5ffb7106..810442bd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs @@ -63,6 +63,14 @@ internal sealed class SqlBrowseSession : IBrowseSession /// private const string ReadOnlySecurityClass = "ViewOnly"; + /// + /// The one SQL type family that maps to while carrying no + /// time of day — SQL Server's and PostgreSQL's date. Ranked below the time-bearing + /// families by ; see for the Oracle + /// exception. + /// + 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 /// reporting , which is /// the same catalog metadata the browse tree already renders per column, on every dialect. - /// Tie-break: FIRST in catalog order. Every dialect's - /// orders by ordinal position (ORDINAL_POSITION / column_id), 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 CreatedUtc/ModifiedUtc pair) it - /// picks the earlier-declared one, which is a preference and not a fact about the data. + /// Ranked, because "maps to DateTime" includes DATE-ONLY types. + /// reports for SQL + /// Server's and PostgreSQL's date as well — and a wide-row read ordered by a date-only column + /// is ORDER BY <date> DESC limited to one row, which ties for every row written on + /// the same day: "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. + /// Falling back to a date-only column is deliberate rather than dropping those types from + /// candidacy: a table whose only date/time column is a date stays bindable by browse (with a + /// default the operator can see and change) instead of becoming un-committable. + /// Tie-break within a rank: FIRST in catalog order. Every dialect's + /// orders by ordinal position (ORDINAL_POSITION / + /// column_id), 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 + /// CreatedUtc/ModifiedUtc pair) it picks the earlier-declared one, which is a preference + /// and not a fact about the data. /// It is a visible, editable DEFAULT, not a hidden binding. The commit writes it into the /// tag's rowSelector.topByTimestamp, where the typed Sql editor shows it and the operator can /// change it — to the other timestamp, or to a whereColumn/whereValue pair entirely. @@ -289,13 +308,42 @@ internal sealed class SqlBrowseSession : IBrowseSession /// 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; + } + + /// + /// Whether a column's declared type family carries a date but no time of day — the case that makes an + /// ORDER BY … DESC single-row read tie for a whole day's rows. + /// + /// The bare SQL type family name, as the catalog reported it. + /// when the family is date-only on this dialect's backend. + /// + /// Provider-qualified, because the spelling is not the meaning. date is date-only on SQL + /// Server and PostgreSQL, but Oracle's DATE carries a time-of-day component — which is + /// why OracleDialect.MapColumnType calls the faithful + /// mapping there rather than a widening. Demoting it on Oracle would change the pick on the backend + /// where DATE is the ordinary timestamp type, for no gain. + /// The check is a browse-time preference and belongs here rather than on + /// : 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. + /// + private bool IsDateOnly(string sqlDataType) + { + if (_dialect.Provider == SqlProvider.Oracle) return false; + return string.Equals(sqlDataType.Trim(), DateOnlyTypeFamily, StringComparison.OrdinalIgnoreCase); } /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs new file mode 100644 index 00000000..270db34a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs @@ -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; + +/// +/// in every respect except that it reports . +/// It exists for exactly one assertion: Oracle's DATE carries a time-of-day component +/// (OracleDialect.MapColumnType 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 DATE column is +/// declared before its TIMESTAMP must still pick the DATE — the ordinary +/// first-in-catalog-order tie-break. +/// Without this control, implementing the ranking provider-blind ("any type family spelled +/// date 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. +/// +public sealed class OracleFlavouredSqliteDialect : ISqlDialect +{ + private readonly SqliteDialect _inner = new(); + + /// The whole point of the type: an Oracle-shaped provider over the SQLite catalog. + public SqlProvider Provider => SqlProvider.Oracle; + + /// + public DbProviderFactory Factory => _inner.Factory; + + /// + public string LivenessSql => _inner.LivenessSql; + + /// + public string SingleRowLimitPrefix => _inner.SingleRowLimitPrefix; + + /// + public string SingleRowLimitSuffix => _inner.SingleRowLimitSuffix; + + /// + public string ListSchemasSql => _inner.ListSchemasSql; + + /// + public string DefaultSchemaSql => _inner.DefaultSchemaSql; + + /// + public string ListTablesSql => _inner.ListTablesSql; + + /// + public string ListColumnsSql => _inner.ListColumnsSql; + + /// + public string QuoteIdentifier(string ident) => _inner.QuoteIdentifier(ident); + + /// + public string ParameterMarker(string baseName) => _inner.ParameterMarker(baseName); + + /// + public string ParameterBindName(string baseName) => _inner.ParameterBindName(baseName); + + /// + public void ConfigureCommand(DbCommand command) => _inner.ConfigureCommand(command); + + /// + public DriverDataType MapColumnType(string sqlDataType) => _inner.MapColumnType(sqlDataType); +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs index 7288b40d..6b2ca8e7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs @@ -458,6 +458,83 @@ public sealed class SqlBrowseSessionTests .ShouldBeGreaterThan(0, "the expected pick must NOT also be the alphabetically first one"); } + /// + /// A date-only column is a candidate of last resort, not the first pick. date maps to + /// on every dialect, so a candidacy rule that only asked the + /// mapped type would commit ORDER BY <date> DESC LIMIT 1 — 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. + /// 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. + /// + [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"); + } + + /// + /// 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. + /// + [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); + } + + /// + /// The falsification control for the ranking: Oracle's DATE carries a time-of-day + /// component, so it is not date-only there and must not be demoted. Same catalog, same columns, + /// only the dialect's differs — and the pick reverts to the + /// ordinary first-in-catalog-order one. + /// A ranking implemented provider-blind ("any family spelled date") passes every other + /// test in this file and fails only here. + /// + [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"); + } + /// /// 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". diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqliteBrowseFixture.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqliteBrowseFixture.cs index b77744a1..9a2318cb 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqliteBrowseFixture.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqliteBrowseFixture.cs @@ -96,6 +96,40 @@ public sealed class SqliteBrowseFixture : IAsyncDisposable /// A value column, sitting BETWEEN the two timestamps. public const string MultiTimeValueColumn = "level"; + /// + /// A table whose FIRST date/time-typed column is date-only, with a genuinely time-bearing one + /// declared after it. A date-only column maps to + /// like any other, so a + /// candidacy rule that only asked the mapped type would order the wide-row read by + /// — one row per DAY, ties broken by storage order. + /// + public const string DateOnlyFirstTable = "DailyTotals"; + + /// The date-only column of , declared first. + public const string DateOnlyFirstColumn = "shift_day"; + + /// A value column, sitting between the two candidates. + public const string DateOnlyFirstValueColumn = "units"; + + /// + /// The time-bearing column of , declared LAST — the expected pick, + /// even though the date-only one comes first in catalog order. + /// + public const string DateOnlyFirstTimeColumn = "logged_at"; + + /// + /// 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. + /// + public const string DateOnlySoleTable = "DayBook"; + + /// The sole (date-only) candidate of . + public const string DateOnlySoleColumn = "book_day"; + + /// A value column. + 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