From 0efee7413adc096a6a690dc76405e48bf2af9968 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 24 Jul 2026 14:31:35 -0400
Subject: [PATCH] fix(sql): SqlQueryPlan snapshots its collections instead of
 aliasing the planner's lists

Code-review Minor: the record typed ParameterNames/Parameters/Members/
SelectedColumns as IReadOnlyList<T> but was handed the planner's live List<T>
instances. SqlQueryPlan documents itself as safe to hold for plan caching, so
an aliased list is a real hazard once the reader starts reusing plans across
polls: IReadOnlyList<T> is downcastable, and one mutation would corrupt every
subsequent poll served by that plan.

Copying at the record boundary fixes it for every call site at once rather
than relying on each planner branch remembering to call AsReadOnly.

Pinned by a test that mutates the caller's lists after construction; verified
load-bearing (reverting the record turns it red).

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
---
 .../SqlQueryPlan.cs                           | 15 ++++++++
 .../SqlQueryPlanImmutabilityTests.cs          | 34 +++++++++++++++++++
 2 files changed, 49 insertions(+)
 create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlQueryPlanImmutabilityTests.cs

diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs
index 0d933d07..f586bbf6 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs
@@ -51,6 +51,21 @@ public sealed record SqlQueryPlan(
     IReadOnlyList<SqlTagDefinition> Members,
     IReadOnlyList<string> SelectedColumns)
 {
+    // A plan is documented as safe to cache and reuse across polls, so it must not stay aliased to the
+    // planner's mutable working lists — an IReadOnlyList<T> holding a List<T> is downcastable, and a
+    // mutation would silently corrupt every later poll that reuses the plan. Snapshot at the boundary.
+    /// <summary>The parameter markers in <see cref="SqlText"/>, aligned with <see cref="Parameters"/>.</summary>
+    public IReadOnlyList<string> ParameterNames { get; } = [.. ParameterNames];
+
+    /// <summary>The values to bind, aligned with <see cref="ParameterNames"/>.</summary>
+    public IReadOnlyList<object?> Parameters { get; } = [.. Parameters];
+
+    /// <summary>The tags this plan feeds, in input order; duplicates preserved.</summary>
+    public IReadOnlyList<SqlTagDefinition> Members { get; } = [.. Members];
+
+    /// <summary>The bare SELECT-list column names, in result-set ordinal order.</summary>
+    public IReadOnlyList<string> SelectedColumns { get; } = [.. SelectedColumns];
+
     /// <summary>
     ///     The key column all members are matched on — <see langword="null"/> for any model but
     ///     <see cref="SqlTagModel.KeyValue"/>. Uniform across <see cref="Members"/> by the group-key invariant,
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlQueryPlanImmutabilityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlQueryPlanImmutabilityTests.cs
new file mode 100644
index 00000000..b76ddd8a
--- /dev/null
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlQueryPlanImmutabilityTests.cs
@@ -0,0 +1,34 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests;
+
+public class SqlQueryPlanImmutabilityTests
+{
+    // A plan is documented as cacheable across polls. If it stayed aliased to the planner's working
+    // List<T>, a consumer downcast could corrupt every later poll that reused it.
+    [Fact]
+    public void Plan_doesNotAliasTheCallersLists()
+    {
+        var names = new List<string> { "@k0" };
+        var values = new List<object?> { "v" };
+        var members = new List<SqlTagDefinition>
+        {
+            new("raw", SqlTagModel.KeyValue, "t") { KeyColumn = "k", KeyValue = "v", ValueColumn = "c" },
+        };
+        var selected = new List<string> { "k", "c" };
+
+        var plan = new SqlQueryPlan(SqlTagModel.KeyValue, "gk", "SELECT 1", names, values, members, selected);
+
+        names.Add("@k1");
+        values.Add("other");
+        members.Add(members[0]);
+        selected.Add("extra");
+
+        plan.ParameterNames.Count.ShouldBe(1);
+        plan.Parameters.Count.ShouldBe(1);
+        plan.Members.Count.ShouldBe(1);
+        plan.SelectedColumns.Count.ShouldBe(2);
+    }
+}