fix(historian): #683 review — bound the StartEnd pick joins to the window; usable picks re-check usability

- SqlServer StartEnd join-back: every pick join also carries the window predicate (forward >= @start AND < @end, reverse > @start AND <= @end). Keyed on the pick time alone a hash/merge/adaptive join read the tag's whole history (review: 43,657 vs 361 logical reads), and parameter sniffing reused that plan for every tag. Forced OPTION (HASH JOIN), 1d@1h on 6M rows: 152,524 -> 6,756 reads. - Usable picks (x/y) re-check the exact u predicate in the join, so a concurrent HistoryUpdate Replace that turned the picked sample Bad/Empty reads as absent (BadNoData), never a Bad row's value under Delta. - StartEndJoinBackTests (integration): 300k history rows either side of a 1 h window, forced HASH and MERGE joins, results identical to the unhinted plan and Start/End correct, session logical reads < 1,000 (183 bounded; 8,055 with the bound removed — the test fails then). - TagIndexTests: the refused-ADD-INDEX test asserts the Warning naming the ALTER. - Historian.md: re-measured table incl. long-history windows (output identical by EXCEPT both ways in all 7 cases) and the forced-join read counts.
2026-10-12 11:10:30 -04:00
parent 910befc84c
commit 4a3b12564a
4 changed files with 185 additions and 15 deletions
@@ -100,7 +100,24 @@ public sealed class TagIndexTests(QuestDbFixture fx)
            await using var alter = conn.CreateCommand(c, QuestDbTables.AddTagIndex(conn.SampleTable));
            await Should.ThrowAsync<Exception>(() => alter.ExecuteNonQueryAsync(Ct)); // precondition: really refused
        }
-        await new QuestDbSchemaBootstrapper(conn, o).EnsureAsync(Ct);
+        var log = new ListLogger();
+        await new QuestDbSchemaBootstrapper(conn, o, log).EnsureAsync(Ct);
+        log.Warnings.ShouldContain(m => m.Contains("Could not add the tag index", StringComparison.Ordinal)
+                                        && m.Contains(QuestDbTables.AddTagIndex(conn.SampleTable), StringComparison.Ordinal));
+    }
+
+    /// <summary>Captures the rendered Warning messages.</summary>
+    private sealed class ListLogger : Microsoft.Extensions.Logging.ILogger
+    {
+        private readonly System.Collections.Concurrent.ConcurrentQueue<string> _warnings = new();
+        public IReadOnlyCollection<string> Warnings => _warnings;
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+        public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => true;
+        public void Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId,
+            TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            if (logLevel == Microsoft.Extensions.Logging.LogLevel.Warning) _warnings.Enqueue(formatter(state, exception));
+        }
    }

    [Fact]
@@ -0,0 +1,123 @@
+using System.Data;
+using Microsoft.Data.SqlClient;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests;
+
+/// <summary>
+///     #683: <see cref="SqlAggregateQueries.StartEnd"/> aggregates the pick times, then fetches each pick by a key join.
+///     The joins must be bounded to the window: keyed on the pick time alone, a hash/merge/adaptive join reads the tag's
+///     whole history (review of 910befc8: 43,657 logical reads vs 361 on a tag with 3M rows outside a 1 d window). The
+///     plan choice depends on estimates, so the test FORCES the worst shape (<c>OPTION (HASH JOIN)</c>) on a tag with long
+///     history either side of the window and asserts the statement reads only window-sized pages — and returns exactly
+///     what the unhinted plan and the data source return.
+/// </summary>
+[Collection(SqlHistorianCollection.Name)]
+public sealed class StartEndJoinBackTests(SqlHistorianFixture fx)
+{
+    private static readonly CancellationToken Ct = CancellationToken.None;
+    private static DateTime At(double s) => ReadTestDb.At(s);
+    private const int HistoryRowsEachSide = 150_000;
+
+    /// <summary>
+    ///     Tag <c>H</c>: one sample a minute over the hour [+0, +3600) of mixed quality (Good, Uncertain, Bad, an outage
+    ///     marker, a string), plus <see cref="HistoryRowsEachSide"/> rows a second before +0 and after +3600.
+    /// </summary>
+    private static async Task<(ReadTestDb Db, int Id, List<(DateTime T, HistorianValue V, uint S)> Window)> SeedAsync(SqlHistorianFixture fx)
+    {
+        var db = await ReadTestDb.CreateAsync(fx);
+        var window = new List<(DateTime T, HistorianValue V, uint S)>();
+        for (var i = 0; i < 60; i++)
+        {
+            var (v, s) = (i % 7) switch
+            {
+                3 => (HistorianValue.FromDouble(i), ReadTestDb.Bad),
+                4 => (HistorianValue.FromDouble(i), ReadTestDb.Uncertain),
+                5 => (HistorianValue.Empty, ReadTestDb.BadNoCommunication),
+                6 => (HistorianValue.FromString("s" + i), ReadTestDb.Good),
+                _ => (HistorianValue.FromDouble(i), ReadTestDb.Good),
+            };
+            window.Add((At(i * 60 + 7), v, s));
+        }
+        var id = await db.SeedAsync("H", window.ToArray());
+        // The history: inserted set-based (a per-row INSERT of 300k rows would dominate the test).
+        await db.ScalarAsync(
+            $"WITH n AS (SELECT TOP ({HistoryRowsEachSide}) ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) AS x " +
+            "FROM sys.all_objects a CROSS JOIN sys.all_objects b) " +
+            $"INSERT {db.Conn.Q("Sample")} (TagId, SourceTimeUtc, ValueKind, NumValue, IntValue, StrValue, StatusCode, ServerTimeUtc) " +
+            $"SELECT {id}, DATEADD(second, -x, CAST('{At(0):yyyy-MM-dd HH:mm:ss.fffffff}' AS datetime2(7))), 3, x, NULL, NULL, 0, SYSUTCDATETIME() FROM n " +
+            $"UNION ALL SELECT {id}, DATEADD(second, x, CAST('{At(3600):yyyy-MM-dd HH:mm:ss.fffffff}' AS datetime2(7))), 3, x, NULL, NULL, 0, SYSUTCDATETIME() FROM n; " +
+            "SELECT 0");
+        await db.ScalarAsync($"UPDATE STATISTICS {db.Conn.Q("Sample")}; SELECT 0");
+        return (db, id, window);
+    }
+
+    /// <summary>
+    ///     Runs the StartEnd SQL (optionally with a query hint) and returns its rows as strings plus the session's logical
+    ///     reads during the statement (<c>sys.dm_exec_sessions.logical_reads</c> before/after — STATISTICS IO messages do
+    ///     not reach SqlClient's InfoMessage here). The two probes add a few reads of their own; the bound is far apart.
+    /// </summary>
+    private static async Task<(List<string> Rows, long LogicalReads)> RunAsync(ReadTestDb db, int id, BucketPlan plan, string? hint)
+    {
+        var sql = SqlAggregateQueries.StartEnd(db.Conn.Q("Sample"), plan.Reversed);
+        if (hint is not null) sql = sql.TrimEnd().TrimEnd(';') + $" OPTION ({hint});";
+        await using var c = await db.Conn.OpenAsync(Ct);
+        async Task<long> SessionReadsAsync()
+        {
+            await using var probe = db.Conn.CreateCommand(c, "SELECT logical_reads FROM sys.dm_exec_sessions WHERE session_id = @@SPID");
+            return Convert.ToInt64(await probe.ExecuteScalarAsync(Ct));
+        }
+        var before = await SessionReadsAsync();
+        await using var cmd = db.Conn.CreateCommand(c, sql);
+        cmd.Parameters.Add(new SqlParameter("@id", SqlDbType.Int) { Value = id });
+        SqlHistorianConnection.AddDateTime2(cmd, "@start", plan.StartUtc);
+        SqlHistorianConnection.AddDateTime2(cmd, "@end", plan.EndUtc);
+        cmd.Parameters.Add(new SqlParameter("@intervalTicks", SqlDbType.BigInt) { Value = plan.IntervalTicks });
+        cmd.Parameters.Add(new SqlParameter("@bucketAdjustTicks", SqlDbType.BigInt) { Value = plan.BucketIndexAdjustTicks });
+        var rows = new List<string>();
+        await using (var r = await cmd.ExecuteReaderAsync(Ct))
+            while (await r.ReadAsync(Ct))
+                rows.Add(string.Join("|", Enumerable.Range(0, r.FieldCount).Select(i => r.IsDBNull(i) ? "NULL" : Convert.ToString(r.GetValue(i), System.Globalization.CultureInfo.InvariantCulture))));
+        return (rows, await SessionReadsAsync() - before);
+    }
+
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task The_pick_joins_read_only_the_window_even_under_a_forced_hash_join(bool reverse)
+    {
+        fx.SkipIfUnavailable();
+        var (db, id, window) = await SeedAsync(fx);
+        var plan = reverse
+            ? BucketPlan.Create(At(3600), At(0), TimeSpan.FromMinutes(10))
+            : BucketPlan.Create(At(0), At(3600), TimeSpan.FromMinutes(10));
+
+        var (plain, _) = await RunAsync(db, id, plan, hint: null);
+        var (hashed, hashReads) = await RunAsync(db, id, plan, hint: "HASH JOIN");
+        var (merged, mergeReads) = await RunAsync(db, id, plan, hint: "MERGE JOIN");
+
+        plain.Count.ShouldBe(6);
+        hashed.ShouldBe(plain);
+        merged.ShouldBe(plain);
+        // Measured: 183 session logical reads bounded vs 8,055 with the window bound removed from the joins (300k history
+        // rows either side; the threshold sits ~5x above the one and ~8x below the other). Every join
+        // bounded ⇒ each input is a window-range seek, whatever the join algorithm.
+        hashReads.ShouldBeGreaterThan(0L, "the session read counter must move (positive control)");
+        hashReads.ShouldBeLessThan(1000, "a hash join's inner input must be bounded to the window");
+        mergeReads.ShouldBeLessThan(1000, "a merge join's inner input must be bounded to the window");
+
+        // And the picks are right: Start / End are each bucket's first / last sample of any quality.
+        var src = db.Source();
+        var start = await src.ReadProcessedAsync("H", reverse ? At(3600) : At(0), reverse ? At(0) : At(3600),
+            TimeSpan.FromMinutes(10), HistoryAggregateType.Start, Ct);
+        var end = await src.ReadProcessedAsync("H", reverse ? At(3600) : At(0), reverse ? At(0) : At(3600),
+            TimeSpan.FromMinutes(10), HistoryAggregateType.End, Ct);
+        var buckets = Enumerable.Range(0, 6).Select(b => window.Skip(b * 10).Take(10).ToList()).ToList();
+        if (reverse) buckets.Reverse();
+        start.Samples.Select(s => s.SourceTimestampUtc).ShouldBe(buckets.Select(b => (DateTime?)b[0].T));
+        end.Samples.Select(s => s.SourceTimestampUtc).ShouldBe(buckets.Select(b => (DateTime?)b[^1].T));
+    }
+}