fix(historian): T10 review — tick-exact bucketing over the full DateTime range; AtTime outage keeps its Bad code

Important 1: bucket offsets are now 100 ns ticks as bigint (DATEDIFF_BIG(microsecond)*10 plus the sub-microsecond tick digits), so a 1601→9999 window no longer overflows (SQL 535) and nothing scales the interval to nanoseconds in C#. Interval wider than the window clamps to one bucket and the ceiling division cannot overflow. Exactness to the tick is unchanged. Important 2 (spec §4 ruling d6afb394): a Bad previous neighbour means the time is inside an outage — null value with that neighbour's Bad code, never Uncertain. Usable previous + Bad next still holds the previous value Uncertain. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-09 13:30:54 -04:00
parent 38d37f5c23
commit 8f859acb2c
4 changed files with 91 additions and 22 deletions
@@ -18,17 +18,21 @@ internal enum SqlAggregateFamily
 /// <summary>
 ///     Push-down T-SQL for processed (aggregate) reads (spec §4 "Processed"). Every query takes
 ///     <c>@id</c> (int TagId), <c>@start</c>/<c>@end</c> (<c>datetime2(7)</c>, the forward window
-///     <c>[start, end)</c>) and <c>@intervalNs</c> (bigint bucket width in nanoseconds), and returns one row
+///     <c>[start, end)</c>) and <c>@intervalTicks</c> (bigint bucket width in 100 ns ticks), and returns one row
 ///     per <b>non-empty</b> bucket. The caller generates every interval and fills the empty ones.
 /// </summary>
 /// <remarks>
 ///     <para>
-///         <b>Bucketing.</b> <c>b = DATEDIFF_BIG(nanosecond, @start, SourceTimeUtc) / @intervalNs</c>. The spec
-///         sketched <c>millisecond</c>, but <c>DATEDIFF</c> counts unit <i>boundaries crossed</i>, not elapsed
-///         time, so with a non-millisecond-aligned <c>@start</c> a sample one tick before a bucket edge would
-///         land in the next bucket. <c>datetime2(7)</c> values are whole multiples of 100 ns, so the nanosecond
-///         boundary count IS the exact elapsed time and the integer division gives start-inclusive /
-///         end-exclusive buckets to the tick. (Range limit: ≈292 years per window.)
+///         <b>Bucketing.</b> <c>b = elapsedTicks / @intervalTicks</c>, where
+///         <c>elapsedTicks = DATEDIFF_BIG(microsecond, @start, t) * 10 + tick-of-microsecond(t) − tick-of-microsecond(@start)</c>
+///         and <c>tick-of-microsecond(x) = (DATEPART(nanosecond, x) / 100) % 10</c>. The spec sketched
+///         <c>millisecond</c>, but <c>DATEDIFF</c> counts unit <i>boundaries crossed</i>, not elapsed time, so with a
+///         non-millisecond-aligned <c>@start</c> a sample one tick before a bucket edge would land in the next
+///         bucket. Counting microsecond boundaries and correcting by the two sub-microsecond tick digits gives
+///         the exact elapsed time in 100 ns ticks (10·(⌊t/10⌋ − ⌊s/10⌋) + t mod 10 − s mod 10 = t − s), so the
+///         integer division is start-inclusive / end-exclusive to the tick. Unlike a nanosecond <c>DATEDIFF_BIG</c>
+///         (which overflows a bigint past ≈292 years, SQL error 535), the microsecond count over the whole
+///         <c>datetime2</c> range (0001–9999, ≈3.2e17 µs, ≈3.2e18 ticks) fits a bigint.
 ///     </para>
 ///     <para>
 ///         <b>Sample classes</b> (status stored bit-for-bit in an <c>int</c>, so the severity bits are the sign
@@ -114,7 +118,8 @@ internal static class SqlAggregateQueries

    /// <summary>The <c>s</c> CTE: the window's samples with bucket index and the usable/good flags.</summary>
    private static string Classified(string sample) =>
-        "WITH s AS (SELECT DATEDIFF_BIG(nanosecond, @start, SourceTimeUtc) / @intervalNs AS b, " +
+        "WITH s AS (SELECT (DATEDIFF_BIG(microsecond, @start, SourceTimeUtc) * 10 " +
+        "+ (DATEPART(nanosecond, SourceTimeUtc) / 100) % 10 - (DATEPART(nanosecond, @start) / 100) % 10) / @intervalTicks AS b, " +
        "SourceTimeUtc, ValueKind, NumValue, IntValue, StrValue, " +
        "CASE WHEN StatusCode >= 0 AND ValueKind <> 0 THEN 1 ELSE 0 END AS u, " +
        "CASE WHEN StatusCode >= 0 AND StatusCode < 1073741824 AND ValueKind <> 0 THEN 1 ELSE 0 END AS g " +
@@ -33,7 +33,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer;
 ///         or a driver's Bad publish) and is Bad whatever its code. Raw returns every sample as stored, outage
 ///         markers included. Aggregates exclude Bad samples from the math, report the interval Uncertain when
 ///         any non-Good sample is present, and count them in PercentBad. AtTime never interpolates across a Bad
-///         neighbour (see <see cref="ReadAtTimeAsync"/>).
+///         neighbour and reports a time inside an outage with the outage's Bad code (see <see cref="ReadAtTimeAsync"/>).
 ///     </para>
 /// </remarks>
 public sealed class SqlServerHistorianDataSource : IHistorianDataSource
@@ -41,6 +41,9 @@ public sealed class SqlServerHistorianDataSource : IHistorianDataSource
    /// <summary>Spec §9.7: a processed read covering more intervals than this is refused.</summary>
    public const int MaxProcessedBuckets = 100_000;

+    /// <summary>Severity + sub-code (top 16 bits) of a status, i.e. the code without info bits.</summary>
+    private const uint StatusCodeBitsMask = 0xFFFF0000u;
+
    private const string SampleColumns = "SourceTimeUtc, ValueKind, NumValue, IntValue, StrValue, StatusCode, ServerTimeUtc";

    private readonly SqlHistorianConnection _conn;
@@ -136,9 +139,11 @@ public sealed class SqlServerHistorianDataSource : IHistorianDataSource
    ///     <list type="bullet">
    ///       <item><c>p</c> at exactly <c>T</c> ⇒ the stored value and status, unchanged.</item>
    ///       <item>No <c>p</c> ⇒ <c>BadNoData</c>, null value.</item>
-    ///       <item><c>p</c> or <c>n</c> Bad (incl. an <c>Empty</c> outage marker) ⇒ <c>UncertainDataSubNormal</c> +
-    ///         Interpolated bits, and NO interpolation across it: the value is <c>p</c>'s held value when <c>p</c> is
-    ///         usable (data held up to the outage), else null (inside an outage).</item>
+    ///       <item><c>p</c> Bad (incl. an <c>Empty</c> outage marker) ⇒ <c>T</c> lies inside an outage: null value
+    ///         carrying <c>p</c>'s Bad status code (info bits stripped) — never better than its source, per the
+    ///         quality contract (spec §4 as amended after the T10 review).</item>
+    ///       <item><c>p</c> usable and <c>n</c> Bad ⇒ <c>p</c>'s value held (stepped, NOT interpolated into the outage)
+    ///         with <c>UncertainDataSubNormal</c> + Interpolated bits.</item>
    ///       <item>Double/Int64 with a usable <c>n</c> ⇒ linear interpolation of <c>NumValue</c> (Int64 rounded to the
    ///         nearest integer, ties away from zero, keeping the type); status <c>Good</c> + Interpolated when both
    ///         neighbours are Good, else <c>UncertainDataSubNormal</c> + Interpolated (never better than the
@@ -208,8 +213,13 @@ public sealed class SqlServerHistorianDataSource : IHistorianDataSource
        if (p.TimeUtc == t)
            return new DataValueSnapshot(p.Value.ToObject(), p.Status, t, serverTime);

-        if (!p.IsUsable || next is { IsUsable: false })
-            return new DataValueSnapshot(p.IsUsable ? p.Value.ToObject() : null,
+        // Inside an outage: report the outage's own Bad code, never an upgraded Uncertain.
+        if (!p.IsUsable)
+            return new DataValueSnapshot(null, p.Status & StatusCodeBitsMask, t, serverTime);
+
+        // Usable previous, Bad next: hold the previous value up to the outage; do not interpolate into it.
+        if (next is { IsUsable: false })
+            return new DataValueSnapshot(p.Value.ToObject(),
                HistorianInfoBits.WithInterpolated(SqlHistorianStatusCodes.UncertainDataSubNormal), t, serverTime);

        if (next is { } n && p.Value.Kind is HistorianValueKind.Double or HistorianValueKind.Int64
@@ -328,8 +338,11 @@ public sealed class SqlServerHistorianDataSource : IHistorianDataSource
        var reverse = endUtc < startUtc;
        var (start, end) = reverse ? (endUtc, startUtc) : (startUtc, endUtc);
        var windowTicks = end.Ticks - start.Ticks;
-        var intervalTicks = interval > TimeSpan.Zero ? interval.Ticks : windowTicks;
-        long bucketCount = windowTicks == 0 ? 0 : (windowTicks + intervalTicks - 1) / intervalTicks;
+        // Everything stays in 100 ns ticks (≤ ≈3.2e18 across 0001–9999, inside a long) — no ns scaling that could
+        // wrap. An interval wider than the window is one bucket, so clamp it to the window; the ceiling division
+        // is written without "+ interval - 1" so a TimeSpan near MaxValue cannot overflow either.
+        var intervalTicks = interval > TimeSpan.Zero ? Math.Min(interval.Ticks, windowTicks) : windowTicks;
+        long bucketCount = windowTicks == 0 ? 0 : windowTicks / intervalTicks + (windowTicks % intervalTicks == 0 ? 0 : 1);
        if (bucketCount > MaxProcessedBuckets)
            throw new ArgumentOutOfRangeException(nameof(interval), bucketCount,
                $"A processed read may cover at most {MaxProcessedBuckets} intervals; this window holds {bucketCount}.");
@@ -353,7 +366,7 @@ public sealed class SqlServerHistorianDataSource : IHistorianDataSource
                cmd.Parameters.Add(new SqlParameter("@id", SqlDbType.Int) { Value = id });
                SqlHistorianConnection.AddDateTime2(cmd, "@start", DateTime.SpecifyKind(start, DateTimeKind.Utc));
                SqlHistorianConnection.AddDateTime2(cmd, "@end", DateTime.SpecifyKind(end, DateTimeKind.Utc));
-                cmd.Parameters.Add(new SqlParameter("@intervalNs", SqlDbType.BigInt) { Value = intervalTicks * 100L });
+                cmd.Parameters.Add(new SqlParameter("@intervalTicks", SqlDbType.BigInt) { Value = intervalTicks });
                await using var reader = await cmd.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false);
                while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
                {
@@ -4,7 +4,11 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions;

 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests;

-/// <summary>AtTime rules (spec §4): exact, linear, stepped, no-prior, Bad neighbour (no bridging).</summary>
+/// <summary>
+///     AtTime rules (spec §4, as amended after the T10 review): exact, linear, stepped, no-prior, and the two
+///     Bad-neighbour cases — inside an outage (Bad previous) ⇒ null with the outage's own Bad code; before an
+///     outage (usable previous, Bad next) ⇒ previous value held, Uncertain. Never bridges.
+/// </summary>
 [Collection(SqlHistorianCollection.Name)]
 public sealed class AtTimeReadTests(SqlHistorianFixture fx)
 {
@@ -75,13 +79,14 @@ public sealed class AtTimeReadTests(SqlHistorianFixture fx)
    }

    [Fact]
-    public async Task A_bad_previous_neighbour_yields_no_value_uncertain()
+    public async Task A_bad_previous_neighbour_is_inside_an_outage_and_keeps_its_bad_code()
    {
        var db = await ReadTestDb.CreateAsync(fx);
        await db.SeedAAsync();
        var s = (await db.Source().ReadAtTimeAsync("A", [At(25)], Ct)).Samples.Single();
-        s.Value.ShouldBeNull(); // NOT 35 (30→40) and NOT 30 (a Bad value)
-        s.StatusCode.ShouldBe(UncertainInterpolated);
+        s.Value.ShouldBeNull();              // NOT 35 (30→40) and NOT 30 (a Bad value)
+        s.StatusCode.ShouldBe(0x80000000u);  // the previous sample's Bad code — never upgraded to Uncertain
+        s.SourceTimestampUtc.ShouldBe(At(25));
    }

    [Fact]
@@ -96,7 +101,7 @@ public sealed class AtTimeReadTests(SqlHistorianFixture fx)
        s[0].Value.ShouldBe(10d);   // bridging 10→30 would give 15
        s[0].StatusCode.ShouldBe(UncertainInterpolated);
        s[1].Value.ShouldBeNull();  // bridging would give 25
-        s[1].StatusCode.ShouldBe(UncertainInterpolated);
+        s[1].StatusCode.ShouldBe(0x80310000u); // inside the outage: BadNoCommunication, as the marker says
        s[2].Value.ShouldBeNull();  // exact hit on the marker: as stored
        s[2].StatusCode.ShouldBe(0x80310000u);
    }
@@ -158,6 +158,52 @@ public sealed class ProcessedReadTests(SqlHistorianFixture fx)
            .Samples.Single().Value.ShouldBe(4);
    }

+    [Fact]
+    public async Task A_whole_range_single_interval_from_1601_to_9999_does_not_overflow()
+    {
+        // 1601-01-01 is the OPC UA minimum DateTime. A nanosecond DATEDIFF_BIG overflows past ≈292 years (SQL 535),
+        // and a ns-scaled interval would wrap a long; both must be gone.
+        var db = await ReadTestDb.CreateAsync(fx);
+        await db.SeedAAsync();
+        var start = new DateTime(1601, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+        var end = new DateTime(9999, 12, 31, 0, 0, 0, DateTimeKind.Utc);
+        var src = db.Source();
+        var avg = (await src.ReadProcessedAsync("A", start, end, TimeSpan.Zero, HistoryAggregateType.Average, Ct)).Samples.Single();
+        avg.Value.ShouldBe(30d); // usable {10, 20, 40, 50}
+        avg.StatusCode.ShouldBe(UncertainCalc);
+        avg.SourceTimestampUtc.ShouldBe(start);
+        (await src.ReadProcessedAsync("A", start, end, TimeSpan.Zero, HistoryAggregateType.Count, Ct))
+            .Samples.Single().Value.ShouldBe(4);
+        // Also a whole-DateTime-range window (0001 → 9999-12-31T23:59:59.9999999).
+        (await src.ReadProcessedAsync("A", DateTime.MinValue, DateTime.MaxValue, TimeSpan.Zero, HistoryAggregateType.Total, Ct))
+            .Samples.Single().Value.ShouldBe(120d);
+        // An interval wider than the window (here TimeSpan.MaxValue) is the same single interval, no overflow.
+        (await src.ReadProcessedAsync("A", start, end, TimeSpan.MaxValue, HistoryAggregateType.Total, Ct))
+            .Samples.Single().Value.ShouldBe(120d);
+        src.GetHealthSnapshot().TotalFailures.ShouldBe(0);
+    }
+
+    [Fact]
+    public async Task A_weekly_interval_over_four_centuries_lands_samples_in_the_right_week()
+    {
+        // Hand-computed (calendar arithmetic, independent of SQL): 1601-01-01 → 2027-01-01 is 155 593 days =
+        // 22 227.57 weeks ⇒ 22 228 intervals (under the 100k cap). 2026-10-09 08:00 is 155 498 days in ⇒ week
+        // index 22 215, which starts Monday 2026-10-05 00:00; every fixture sample (t0 … t0+40 s) is in it.
+        var db = await ReadTestDb.CreateAsync(fx);
+        await db.SeedAAsync();
+        var start = new DateTime(1601, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+        var end = new DateTime(2027, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+        var s = (await db.Source().ReadProcessedAsync("A", start, end, TimeSpan.FromDays(7), HistoryAggregateType.Average, Ct)).Samples;
+
+        s.Count.ShouldBe(22_228);
+        var hit = s[22_215];
+        hit.SourceTimestampUtc.ShouldBe(new DateTime(2026, 10, 5, 0, 0, 0, DateTimeKind.Utc));
+        hit.Value.ShouldBe(30d);
+        hit.StatusCode.ShouldBe(UncertainCalc);
+        s.Where((_, i) => i != 22_215).ShouldAllBe(x => x.StatusCode == NoDataCalc);
+        s[^1].SourceTimestampUtc.ShouldBe(new DateTime(2026, 12, 28, 0, 0, 0, DateTimeKind.Utc)); // partial last week
+    }
+
    [Fact]
    public async Task A_reverse_window_is_computed_forward_and_returned_descending()
    {