The gap
ComposeCompiler builds every composed-panel query by appending SQL text while allocating positional parameters through ParamList, and the correspondence between the two is maintained by hand at each site. Nothing checks it. A site that binds the same value twice gets a fresh ordinal for the second bind, the SQL it writes is valid, the query runs, and every ordinal allocated after it shifts by one.
That is not hypothetical shape-spotting. It is the obvious wrong way to do the thing #3202 did, and I confirmed by mutation that it survives the suite.
What was removed, and what stayed green
In #3202's ComposeCompiler.CompileAnnotation, a server-local annotation source needs the panel's server array in two places: the outer f.server_name = ANY($3) predicate and the offset-join subquery that scopes the DISTINCT ON. The shipped code reuses the already-allocated $3. The tempting alternative is a second p.AddTextArray(context.Servers!), which compiles and reads correctly.
I applied that mutation and progressively removed the assertions guarding it:
| assertions left in place |
duplicate bind detected? |
| everything as shipped |
yes — by Assert.Contains("AND server_name = ANY($3)", ...) |
ordinal blinded to ANY($ — count assert kept |
yes — by Assert.Equal(3, scoped.Parameters.Count) |
| ordinal blinded and count assert removed |
no. Total: 273, Failed: 0 |
The third row is the finding. With two assertions in one new test removed, a duplicate parameter bind in the compose compiler is caught by nothing in 273 tests — and that count includes the whole of DarlingComposeTests, which is the suite that exists to hold this compiler's safety invariants.
Why DarlingComposeTests' own ordinal assertions do not see it
They are per-statement, and they assert the ordinals of the statement they were written for.
CompileAnnotations_EmitsSchemaQualifiedCollect_WindowAndServerBound_Capped does assert Assert.Equal(3, one.Parameters.Count) — but it compiles the deadlocks source, which is UTC-framed and therefore never takes the offset join, so the mutation cannot reach it. CompileAnnotations_EachSource_SelectsItsCatalogTimeAndLabelColumns does cover default_trace_events, but asserts only the ts and label expressions, not any parameter count. So the one test that counts parameters looks at a statement the bug cannot occur in, and the one test that looks at the affected statement does not count parameters.
That is the general shape of the problem, and it is why fixing it by adding another per-site assertion would not close it: an ordinal assertion on one statement is evidence about that statement. It says nothing about the next site someone adds, and the realistic regression is exactly a new site — a new annotation source, a new measure route, a new filter form — written by someone who binds a value they already had.
The instrument that would close it
ComposeCompiled already carries both halves of the invariant: Sql and Parameters. So the check is available without new plumbing, and it is a property of every compiled statement rather than of any one of them. Roughly:
- Every
$n appearing in Sql satisfies 1 <= n <= Parameters.Count.
- Every ordinal in
1..Parameters.Count appears at least once in Sql — an allocated-but-unreferenced parameter is the signature of a duplicate bind, since the duplicate is allocated and the SQL still cites the original.
- Applied across a corpus of compiled statements: every measure route, every annotation source, the server-scoped and fleet-wide variants of each, and the filter and variable forms.
The second bullet is the one that catches this specific mutation, and it is worth stating why it is the right discriminator rather than "count the $ns and compare": a duplicate bind leaves the ordinal count HIGHER than the highest ordinal used, so a max-based check passes while a coverage-based check fails. It fails toward the worse label.
Two cautions for whoever writes it. First, it must be proven to find the statements — a corpus sweep that compiles nothing reports a clean bill of health, so assert a floor on how many statements it examined, the way ServerLocalReadFrameDisciplineTests and CreationTimeClockFrameDisciplineTests both do. Second, red-proof it by the mutation above rather than by inspection: the whole reason this issue exists is that a guard which looked like it covered the compiler did not.
Provenance
Found while red-proofing #3202 (fix/3198-trace-event-time-frame), by crippling two guards at once rather than one at a time. Single-mutation testing would have missed it — with either assertion present the mutation is caught, so each looks individually sufficient and neither is revealed as the only thing holding the property. Relevant files:
Darling/PerformanceMonitor.Darling.Service/Compose/ComposeCompiler.cs — ParamList, CompileAnnotation, CompileDarling/Darling.Tests/DarlingComposeTests.cs — the existing per-statement ordinal assertionsDarling/Darling.Tests/ServerLocalReadFrameDisciplineTests.cs — CompiledAnnotationSql_ScopesTheOffsetJoin_ToThePanelsOwnServerArray, the two assertions in question
Not claimed
No shipped compose statement has a duplicate bind today; I did not find a live defect. This is a missing check, and the argument for it is that the class is reachable, silent, and currently unguarded — not that it has already bitten.
The gap
ComposeCompilerbuilds every composed-panel query by appending SQL text while allocating positional parameters throughParamList, and the correspondence between the two is maintained by hand at each site. Nothing checks it. A site that binds the same value twice gets a fresh ordinal for the second bind, the SQL it writes is valid, the query runs, and every ordinal allocated after it shifts by one.That is not hypothetical shape-spotting. It is the obvious wrong way to do the thing #3202 did, and I confirmed by mutation that it survives the suite.
What was removed, and what stayed green
In #3202's
ComposeCompiler.CompileAnnotation, a server-local annotation source needs the panel's server array in two places: the outerf.server_name = ANY($3)predicate and the offset-join subquery that scopes theDISTINCT ON. The shipped code reuses the already-allocated$3. The tempting alternative is a secondp.AddTextArray(context.Servers!), which compiles and reads correctly.I applied that mutation and progressively removed the assertions guarding it:
Assert.Contains("AND server_name = ANY($3)", ...)ANY($— count assert keptAssert.Equal(3, scoped.Parameters.Count)Total: 273, Failed: 0The third row is the finding. With two assertions in one new test removed, a duplicate parameter bind in the compose compiler is caught by nothing in 273 tests — and that count includes the whole of
DarlingComposeTests, which is the suite that exists to hold this compiler's safety invariants.Why
DarlingComposeTests' own ordinal assertions do not see itThey are per-statement, and they assert the ordinals of the statement they were written for.
CompileAnnotations_EmitsSchemaQualifiedCollect_WindowAndServerBound_Cappeddoes assertAssert.Equal(3, one.Parameters.Count)— but it compiles thedeadlockssource, which is UTC-framed and therefore never takes the offset join, so the mutation cannot reach it.CompileAnnotations_EachSource_SelectsItsCatalogTimeAndLabelColumnsdoes coverdefault_trace_events, but asserts only thetsand label expressions, not any parameter count. So the one test that counts parameters looks at a statement the bug cannot occur in, and the one test that looks at the affected statement does not count parameters.That is the general shape of the problem, and it is why fixing it by adding another per-site assertion would not close it: an ordinal assertion on one statement is evidence about that statement. It says nothing about the next site someone adds, and the realistic regression is exactly a new site — a new annotation source, a new measure route, a new filter form — written by someone who binds a value they already had.
The instrument that would close it
ComposeCompiledalready carries both halves of the invariant:SqlandParameters. So the check is available without new plumbing, and it is a property of every compiled statement rather than of any one of them. Roughly:$nappearing inSqlsatisfies1 <= n <= Parameters.Count.1..Parameters.Countappears at least once inSql— an allocated-but-unreferenced parameter is the signature of a duplicate bind, since the duplicate is allocated and the SQL still cites the original.The second bullet is the one that catches this specific mutation, and it is worth stating why it is the right discriminator rather than "count the
$ns and compare": a duplicate bind leaves the ordinal count HIGHER than the highest ordinal used, so a max-based check passes while a coverage-based check fails. It fails toward the worse label.Two cautions for whoever writes it. First, it must be proven to find the statements — a corpus sweep that compiles nothing reports a clean bill of health, so assert a floor on how many statements it examined, the way
ServerLocalReadFrameDisciplineTestsandCreationTimeClockFrameDisciplineTestsboth do. Second, red-proof it by the mutation above rather than by inspection: the whole reason this issue exists is that a guard which looked like it covered the compiler did not.Provenance
Found while red-proofing #3202 (
fix/3198-trace-event-time-frame), by crippling two guards at once rather than one at a time. Single-mutation testing would have missed it — with either assertion present the mutation is caught, so each looks individually sufficient and neither is revealed as the only thing holding the property. Relevant files:Darling/PerformanceMonitor.Darling.Service/Compose/ComposeCompiler.cs—ParamList,CompileAnnotation,CompileDarling/Darling.Tests/DarlingComposeTests.cs— the existing per-statement ordinal assertionsDarling/Darling.Tests/ServerLocalReadFrameDisciplineTests.cs—CompiledAnnotationSql_ScopesTheOffsetJoin_ToThePanelsOwnServerArray, the two assertions in questionNot claimed
No shipped compose statement has a duplicate bind today; I did not find a live defect. This is a missing check, and the argument for it is that the class is reachable, silent, and currently unguarded — not that it has already bitten.