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
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user