From cb7fdf6682dfa94c788e15a91f8b93e2f9faca70 Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 18:45:41 -0400 Subject: [PATCH 1/9] Fix CTE inner-param slot skew in AssembledPlan site-parameter walk (#305) BuildSiteParamsMap and BuildParamConditionalMap advanced the global parameter offset by 0 for CteDefinition sites (their Clause is null), even though ChainAnalyzer allocates the CTE's inner params ahead of all clause params. Every param-bearing clause after a param-bearing With() then resolved its carrier P-fields short by the inner-param count: the outer Where assigned P0 (the CTE's slot) and P1 was never assigned, tripping the QRY037 self-check and making inner+outer parameterized CTE chains unbuildable. Advance the offset by the matching CteDef's inner-param count, matched by CTE short name (same first-match rule as EmitCteDefinition). Add a generation guard: the issue #305 chain shape produces no QRY037 and the generated interceptors assign both P0 and P1. --- .../plan.md | 99 +++++++++++++++++++ .../workflow.md | 97 ++++++++++++++++++ src/Quarry.Generator/IR/AssembledPlan.cs | 42 ++++++++ .../Generation/CarrierGenerationTests.cs | 51 ++++++++++ 4 files changed, 289 insertions(+) create mode 100644 _sessions/305-fix-cte-outer-param-assignment/plan.md create mode 100644 _sessions/305-fix-cte-outer-param-assignment/workflow.md diff --git a/_sessions/305-fix-cte-outer-param-assignment/plan.md b/_sessions/305-fix-cte-outer-param-assignment/plan.md new file mode 100644 index 00000000..38ec7a25 --- /dev/null +++ b/_sessions/305-fix-cte-outer-param-assignment/plan.md @@ -0,0 +1,99 @@ +# Plan: 305-fix-cte-outer-param-assignment + +## Background + +A CTE chain combining a captured param inside `With(...)` with a captured param in an +outer clause fails the build with QRY037: the carrier declares the outer param's field +(`P1`) but no interceptor body assigns it. + +The chain's parameter model is correct end-to-end: `ChainAnalyzer` places CTE inner params +at global slots `[CteDef.ParameterOffset, ParameterOffset + InnerParameters.Count)` and +remaps outer clause params to follow them; `SqlAssembler` rebases inner placeholder +rendering with `paramBaseOffset: cte.ParameterOffset`; the carrier declares one `P{n}` +field per global slot. The defect is confined to `AssembledPlan`'s cached site-parameter +walk, which every emitter consults to decide *which* `P{n}` a clause interceptor assigns. + +## Key concept: the offset walk + +`AssembledPlan.BuildSiteParamsMap()` iterates `GetClauseEntries()` in chain order, +maintaining `globalParamOffset` — the first global slot owned by the current site. Each +entry advances the offset by the number of slots it owns (translated clause params, +UpdateSetPoco columns, set-operation operand params, UpdateSetAction params, Select +projection params). A `CteDefinition` entry owns `InnerParameters.Count` slots but has +`Clause == null` (CteDefinition is not clause-bearing in `CallSiteTranslator`), so it +falls through every case and advances the offset by 0. Every param-bearing clause after +it then receives an offset short by the total inner-param count, and +`CarrierEmitter.EmitCarrierClauseBody` emits `__c.P{offset+i} = ...` against the wrong +slots: the outer Where writes `P0` (stomping the CTE copy) and `P1` stays unassigned → +QRY037. Zero inner params ⇒ zero offset error ⇒ why inner-only / outer-only / two-inner +shapes all work. + +`BuildParamConditionalMap()` walks the same way via `GetClauseParamCount` and has the +identical gap. Its consumer (`PipelineOrchestrator.RewriteMySqlBindMarkers`) treats +missing keys as unconditional/active, so CTE slots need no map entries — only the offset +must advance so post-CTE clause params are keyed at their true slots. + +## Algorithm: the fix + +In both walks, add a `CteDefinition` case that advances the offset by the matching +`CteDef.InnerParameters.Count`, matched by CTE short name — the same first-match-by-name +rule `TransitionBodyEmitter.EmitCteDefinition` uses (duplicate names are already compile +errors via QRY082; a failed-analysis CTE has no CteDef and the chain is already a QRY080 +error, so advancing 0 there is consistent): + +```csharp +else if (clause.Site.Kind == Models.InterceptorKind.CteDefinition) +{ + var cteName = CteNameHelpers.ExtractShortName( + clause.Site.Bound.Raw.CteEntityTypeName ?? clause.Site.EntityTypeName); + foreach (var cte in Plan.CteDefinitions) + { + if (cte.Name == cteName) + { + globalParamOffset += cte.InnerParameters.Count; + break; + } + } +} +``` + +Placement: before the generic `clause.Site.Clause != null` case is not required +(CteDefinition sites always have null Clause) but the branch order must keep the existing +cases untouched. `BuildParamConditionalMap` needs the same branch inline in its loop +(alongside the existing set-operation `continue` case) because `GetClauseParamCount` is +static and has no access to `Plan.CteDefinitions`. + +## Steps + +- [x] **Step 1 — Fix `AssembledPlan` offset walk + generation guard test.** + Modify `src/Quarry.Generator/IR/AssembledPlan.cs`: add the CteDefinition advance to + `BuildSiteParamsMap` and `BuildParamConditionalMap` (as an inline branch mirroring the + set-op handling). Add a generation test in + `src/Quarry.Tests/Generation/CarrierGenerationTests.cs` (QRY037 region): compile the + issue #305 chain shape (captured inner + captured outer param) via the real generator + pipeline; assert no QRY037 diagnostic and that the generated interceptor source assigns + both `__c.P0` (CTE copy in the With interceptor) and `__c.P1` (outer Where interceptor). + Run the full Quarry.Tests suite. Commit. + Tests: new generation test; existing suite green (esp. `CrossDialectCteTests`, + `MySqlBindOrderGenerationTests`, QRY037 self-check tests). + +- [ ] **Step 2 — Cross-dialect SQL + execution test.** + Add to `src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs`: inner+outer captured + params (`With(orders => orders.Where(o => o.Total > threshold)).FromCte() + .Where(o => o.OrderId >= minId).Select(...)`) built on all four dialects with + `Prepare()`, `AssertDialects` on the rendered WITH statement (`@p0/@p1`, `$1/$2`, `?/?`, + `@p0/@p1`), then execute-and-verify rows on all four backends (expect only order 3: + `(3, 150.00m)`). Run suite. Commit. + Tests: the new cross-dialect test itself. + +- [ ] **Step 3 — MySQL bind-order execution pin.** + Add `ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter` to + `src/Quarry.Tests/Integration/MySqlIntegrationTests.cs` exactly as specified in issue + #305 (companion to the three #304 `ParameterizedCte*` pins; same seed data). Also update + the now-stale comments in `ParameterizedCteFilter_OnMySQL_BindsInnerParamsBeforeOuter` / + `ParameterizedCteTwoInnerParams_OnMySQL_BindsBothInTextOrder` that describe QRY037 as + blocking this shape. Run suite. Commit. + Tests: the new MySQL integration test. + +Dependencies: Step 1 unblocks 2 and 3 (the shape doesn't build before the fix). Steps 2 +and 3 are independent of each other. diff --git a/_sessions/305-fix-cte-outer-param-assignment/workflow.md b/_sessions/305-fix-cte-outer-param-assignment/workflow.md new file mode 100644 index 00000000..b6cdaf73 --- /dev/null +++ b/_sessions/305-fix-cte-outer-param-assignment/workflow.md @@ -0,0 +1,97 @@ +# Workflow: 305-fix-cte-outer-param-assignment + +## Config +platform: github +base-branch: master + +## State +phase: IMPLEMENT +status: active +issue: #305 +pr: + +## Problem Statement +QRY037 build failure for CTE chains that combine a captured parameter inside the +`With(...)` lambda with a captured parameter in an outer clause (e.g. `Where` +after `FromCte()`). The generated carrier declares the outer parameter's field +(`P1`) but no clause interceptor body assigns it, so the generator's self-check +(`DiagnosticDescriptors.CarrierParameterFieldUnassigned`) fails the build. The +chain shape is valid SQL on all four dialects but is currently unusable. + +Repro (from issue): +```csharp +decimal threshold = 100.00m; +int minId = 2; +var rows = await db.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) // carrier field never assigned -> QRY037 + .Select(o => (o.OrderId, o.Total)) + .ExecuteFetchAllAsync(); +``` + +Suspected root cause (from issue): CTE parameter rebasing in `ChainAnalyzer` +prepends inner-lambda params to the outer parameter list with sequential +`GlobalIndex`, but the clause-interceptor parameter-assignment emission does not +apply the rebase offset for outer clauses — with zero inner params the offset is +zero, which is why outer-only chains work. + +Baseline test status: all green at 7bb0e35 — Quarry.Tests 3281 passed, Quarry.Migration.Tests 201 passed, Quarry.Analyzers.Tests 146 passed, 0 failed, 0 skipped (Docker available; all 4 dialects executed). Build emits pre-existing CS0219 `__colShift` warnings from CrossDialectUpdateTests interceptors — warnings only, addressed by an unrelated uncommitted change in the main repo. + +## Decisions +- 2026-07-02: Root cause fixed at `AssembledPlan.BuildSiteParamsMap`/`BuildParamConditionalMap` + (offset walk skips CteDefinition entries), NOT in ChainAnalyzer/SqlAssembler as the issue + guessed — those are correct. CTE match by short name via `CteNameHelpers.ExtractShortName`, + mirroring `TransitionBodyEmitter` (first match; duplicates already QRY082, failed CTEs + already QRY080). +- 2026-07-02: Design + 3-step plan **provisionally approved** (user AFK at AskUserQuestion + timeout; proceeded per autonomous-operation guidance with the recommended option). User may + revisit with "go back to DESIGN". +- 2026-07-02: Tests: (1) generation guard — no QRY037 + interceptor assigns P0 and P1; + (2) cross-dialect SQL + execution in CrossDialectCteTests; (3) MySQL bind-order pin + `ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter` exactly as specified + in issue #305. + +## Working Notes +- **Root cause (confirmed by code trace, DESIGN 2026-07-02):** not in ChainAnalyzer or + SqlAssembler — both are correct. `ChainAnalyzer` inserts CTE inner params into the + plan's global parameter list at `CteDef.ParameterOffset` and remaps outer clause + params to follow them (`ChainAnalyzer.cs:977` runs after the CTE loop advanced + `paramGlobalIndex`). SQL rendering rebase is also correct (`SqlAssembler` uses + `paramBaseOffset: cte.ParameterOffset`). The defect is in + `AssembledPlan.BuildSiteParamsMap` (`AssembledPlan.cs:187`) and + `BuildParamConditionalMap` (`AssembledPlan.cs:238`): both walk clause entries + accumulating `globalParamOffset`, but a `CteDefinition` entry has `Clause == null` + (CteDefinition is not clause-bearing — `CallSiteTranslator.IsClauseBearingKind`, + line 779) and matches no offset-advance case, so it contributes 0 to the offset + despite occupying `InnerParameters.Count` slots in `ChainParameters`. Every + param-bearing clause after the CTE then gets an offset short by the inner-param + count. `CarrierEmitter.EmitCarrierClauseBody` (line 264, 300) uses that offset to + emit `__c.P{offset+i} = ...`, so the outer Where writes `P0` (double-assigning the + CTE's slot) and `P1` is never assigned → QRY037. With zero inner params the offset + error is zero — exactly why outer-only and inner-only chains work. +- The CTE-slot assignments themselves don't use this map: `TransitionBodyEmitter. + EmitCteDefinition` uses `cteDef.ParameterOffset` directly (name-matched via + `CteNameHelpers.ExtractShortName`, first-match; duplicates rejected by QRY082). + The fix should mirror that name-matching. +- `BuildParamConditionalMap` has the same walk and the same gap. Its consumer + (`PipelineOrchestrator.RewriteMySqlBindMarkers:641`) treats missing keys as + unconditional/active, so unmapped CTE slots are fine, but misaligned keys would + mislabel slots for conditional-clause + parameterized-CTE chains (currently + unreachable — QRY037 blocks all such shapes — but must be fixed consistently). +- All emitter paths funnel through `AssembledPlan.GetSiteParams` (ClauseBodyEmitter, + JoinBodyEmitter, CarrierEmitter, TerminalEmitHelpers) — a single fix point in + AssembledPlan covers every consumer. +- Generator llm.md's QRY table skips QRY037 (jumps QRY036→QRY040) — pre-existing doc + gap, out of scope here. +- Main repo (Quarry-master) has an uncommitted, unrelated change to + `src/Quarry.Generator/CodeGen/CarrierEmitter.cs` (Patch-chain `__colShift` + CS0219 avoidance). It is NOT part of this branch; worktree branched from clean + HEAD 7bb0e35. May cause a merge conflict later if it lands while this branch + also touches CarrierEmitter. + +## Suspend State + +## Session Log +| Date | Phases | Summary | +|------|--------|---------| +| 2026-07-02 | INTAKE, DESIGN, PLAN | Loaded issue #305, created worktree + branch, baseline all green (3628 tests). Traced root cause to AssembledPlan offset walk. Design+plan provisionally approved (user AFK); wrote plan.md; entered IMPLEMENT. | diff --git a/src/Quarry.Generator/IR/AssembledPlan.cs b/src/Quarry.Generator/IR/AssembledPlan.cs index c0a1222a..a5b6d037 100644 --- a/src/Quarry.Generator/IR/AssembledPlan.cs +++ b/src/Quarry.Generator/IR/AssembledPlan.cs @@ -210,6 +210,21 @@ public IReadOnlyList GetClauseEntries() globalParamOffset += Plan.SetOperations[setOpIndex].Operand.Parameters.Count; setOpIndex++; } + else if (clause.Site.Kind == Models.InterceptorKind.CteDefinition) + { + // A CteDefinition site owns the chain slots [CteDef.ParameterOffset, + // ParameterOffset + InnerParameters.Count) — ChainAnalyzer merges the + // inner chain's params into the outer parameter list ahead of all clause + // params. The site's Clause is null (CteDefinition is not clause-bearing), + // so without this branch the walk would advance by 0 and every param- + // bearing clause after the CTE would resolve its P-fields short by the + // inner-param count (issue #305: outer Where assigned P0 instead of P1, + // tripping the QRY037 self-check). Match by CTE short name — the same + // first-match rule TransitionBodyEmitter.EmitCteDefinition uses + // (duplicates are QRY082 errors; a failed-analysis CTE has no CteDef and + // the chain is already a QRY080 error, so advancing 0 is consistent). + globalParamOffset += GetCteInnerParamCount(clause.Site); + } else if (clause.Site.Clause != null) globalParamOffset += clause.Site.Clause.Parameters.Count; else if (clause.Site.Kind == Models.InterceptorKind.UpdateSetAction && clause.Site.Bound.Raw.SetActionParameters != null) @@ -250,6 +265,15 @@ public IReadOnlyList GetClauseEntries() setOpIndex++; continue; } + if (clause.Site.Kind == Models.InterceptorKind.CteDefinition) + { + // Skip the CTE's inner-param slots without adding map entries: inner + // chains have no conditional clauses, and consumers treat missing keys + // as unconditional/active. The offset must still advance so post-CTE + // clause params are keyed at their true slots (see BuildSiteParamsMap). + globalOffset += GetCteInnerParamCount(clause.Site); + continue; + } var paramCount = GetClauseParamCount(clause); for (int i = 0; i < paramCount; i++) map[globalOffset + i] = (clause.IsConditional, clause.BitIndex); @@ -258,6 +282,24 @@ public IReadOnlyList GetClauseEntries() return map; } + /// + /// Returns the number of chain parameter slots owned by a CteDefinition site: the + /// inner-param count of the CteDef whose name matches the site's DTO short name + /// (first match, mirroring TransitionBodyEmitter.EmitCteDefinition), or 0 when no + /// CteDef matched (inner chain failed analysis — the chain already carries QRY080). + /// + private int GetCteInnerParamCount(TranslatedCallSite site) + { + var cteName = CteNameHelpers.ExtractShortName( + site.Bound.Raw.CteEntityTypeName ?? site.EntityTypeName); + foreach (var cte in Plan.CteDefinitions) + { + if (cte.Name == cteName) + return cte.InnerParameters.Count; + } + return 0; + } + private static int GetClauseParamCount(ChainClauseEntry clause) { if (clause.Site.Kind == Models.InterceptorKind.UpdateSetPoco && clause.Site.UpdateInfo != null) diff --git a/src/Quarry.Tests/Generation/CarrierGenerationTests.cs b/src/Quarry.Tests/Generation/CarrierGenerationTests.cs index 49202269..b883b663 100644 --- a/src/Quarry.Tests/Generation/CarrierGenerationTests.cs +++ b/src/Quarry.Tests/Generation/CarrierGenerationTests.cs @@ -4485,6 +4485,57 @@ public string Test() + string.Join("; ", qry037.Select(d => d.GetMessage()))); } + [Test] + public void CteInnerAndOuterCapturedParams_NoQRY037_AssignsBothPFields() + { + // Issue #305: a captured param inside the With() lambda AND a captured param + // in the outer Where used to trip QRY037 — AssembledPlan's site-parameter walk + // advanced the global offset by 0 for the CteDefinition site (its Clause is + // null), so the outer Where interceptor was emitted against P0 (the CTE's slot) + // and P1 was never assigned. The fix advances the offset by the CteDef's + // inner-param count; both P-fields must now be assigned, and the chain must + // build cleanly. + var source = SharedSchema + @" +[QuarryContext(Dialect = SqlDialect.SQLite)] +public partial class TestDbContext : QuarryContext +{ + public partial IEntityAccessor Orders(); +} + +public static class Queries +{ + public static async Task Test(TestDbContext db) + { + decimal threshold = 100.00m; + int minId = 2; + await db.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .ExecuteFetchAllAsync(); + } +} +"; + var compilation = CreateCompilation(source); + var (result, diagnostics) = RunGeneratorWithDiagnostics(compilation); + + var qry037 = diagnostics.Where(d => d.Id == "QRY037").ToList(); + Assert.That(qry037, Is.Empty, + "Inner+outer captured CTE params must not trip QRY037. Diagnostics: " + + string.Join("; ", qry037.Select(d => d.GetMessage()))); + + var tree = result.GeneratedTrees + .FirstOrDefault(t => t.FilePath.EndsWith(".g.cs") && t.FilePath.Contains(".Interceptors.")); + Assert.That(tree, Is.Not.Null, "Interceptor file must be generated for the CTE chain"); + var code = tree!.GetText().ToString(); + + Assert.That(code, Does.Match(@"__c\.P0\s*="), + "The With interceptor must assign the CTE's inner captured param to P0"); + Assert.That(code, Does.Match(@"__c\.P1\s*="), + "The outer Where interceptor must assign its captured param to P1 (the " + + "slot after the CTE's inner param), not stomp P0"); + } + /// /// Helper to build a synthetic CarrierPlan with the given carrier name and fields. /// Bypasses the full pipeline so the gap-detection logic can be tested in isolation. From 0e96ee1ca1e15f98dc033bd83267062e553c713e Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 18:50:43 -0400 Subject: [PATCH 2/9] Add cross-dialect SQL + execution test for inner+outer parameterized CTE (#305) Pins the previously-unbuildable shape on all four dialects: exact WITH statement rendering (inner param at slot 0, outer Where param at slot 1) plus execute-and-verify against all four backends. --- .../plan.md | 2 +- .../ManifestOutput/quarry-manifest.mysql.md | 17 ++++- .../quarry-manifest.postgresql.md | 17 ++++- .../ManifestOutput/quarry-manifest.sqlite.md | 17 ++++- .../quarry-manifest.sqlserver.md | 17 ++++- .../SqlOutput/CrossDialectCteTests.cs | 63 +++++++++++++++++++ 6 files changed, 124 insertions(+), 9 deletions(-) diff --git a/_sessions/305-fix-cte-outer-param-assignment/plan.md b/_sessions/305-fix-cte-outer-param-assignment/plan.md index 38ec7a25..47fc1ac6 100644 --- a/_sessions/305-fix-cte-outer-param-assignment/plan.md +++ b/_sessions/305-fix-cte-outer-param-assignment/plan.md @@ -77,7 +77,7 @@ static and has no access to `Plan.CteDefinitions`. Tests: new generation test; existing suite green (esp. `CrossDialectCteTests`, `MySqlBindOrderGenerationTests`, QRY037 self-check tests). -- [ ] **Step 2 — Cross-dialect SQL + execution test.** +- [x] **Step 2 — Cross-dialect SQL + execution test.** Add to `src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs`: inner+outer captured params (`With(orders => orders.Where(o => o.Total > threshold)).FromCte() .Where(o => o.OrderId >= minId).Select(...)`) built on all four dialects with diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md index ad5b5a75..c6c36ff6 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md @@ -5109,6 +5109,19 @@ WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `Ord --- +### With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `OrderDate`, `Notes` FROM `orders` WHERE `Total` > ?) SELECT `OrderId`, `Total` FROM `Order` WHERE `OrderId` >= ? +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `int` | + +--- + ### With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5228,7 +5241,7 @@ SELECT `UserId`, `UserName`, `Email`, `IsActive`, `CreatedAt`, `LastLogin` FROM | Metric | Count | |--------|------:| -| Total discovered | 573 | +| Total discovered | 574 | | Skipped (errors) | 0 | | Consolidated (deduped) | 99 | -| Rendered | 474 | +| Rendered | 475 | diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md index 55192114..9304f3e5 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md @@ -5229,6 +5229,19 @@ WITH "OrderSummaryDto" AS (SELECT "OrderId", "Total", "Status" FROM "orders" WHE --- +### With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "OrderDate", "Notes" FROM "orders" WHERE "Total" > $1) SELECT "OrderId", "Total" FROM "Order" WHERE "OrderId" >= $2 +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `int` | + +--- + ### With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5280,7 +5293,7 @@ SELECT "UserId", "UserName", "Email", "IsActive", "CreatedAt", "LastLogin" FROM | Metric | Count | |--------|------:| -| Total discovered | 601 | +| Total discovered | 602 | | Skipped (errors) | 0 | | Consolidated (deduped) | 117 | -| Rendered | 484 | +| Rendered | 485 | diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md index 872fd216..43646a1d 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md @@ -5576,6 +5576,19 @@ WITH "OrderSummaryDto" AS (SELECT "OrderId", "Total", "Status" FROM "orders" WHE --- +### With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "OrderDate", "Notes" FROM "orders" WHERE "Total" > @p0) SELECT "OrderId", "Total" FROM "Order" WHERE "OrderId" >= @p1 +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `int` | + +--- + ### With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5619,7 +5632,7 @@ WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "Ord | Metric | Count | |--------|------:| -| Total discovered | 706 | +| Total discovered | 707 | | Skipped (errors) | 0 | | Consolidated (deduped) | 193 | -| Rendered | 513 | +| Rendered | 514 | diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md index 3b7cd5de..b73a9f4a 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md @@ -5049,6 +5049,19 @@ WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [Ord --- +### With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [OrderDate], [Notes] FROM [orders] WHERE [Total] > @p0) SELECT [OrderId], [Total] FROM [Order] WHERE [OrderId] >= @p1 +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `int` | + +--- + ### With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5092,7 +5105,7 @@ WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [Ord | Metric | Count | |--------|------:| -| Total discovered | 566 | +| Total discovered | 567 | | Skipped (errors) | 0 | | Consolidated (deduped) | 101 | -| Rendered | 465 | +| Rendered | 466 | diff --git a/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs b/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs index 3afb8b49..b13a93d2 100644 --- a/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs +++ b/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs @@ -175,6 +175,69 @@ public async Task Cte_FromCte_CapturedParam() Assert.That(ssResults2[0], Is.EqualTo((1, 250.00m))); } + /// + /// Regression test for issue #305: a captured param inside the With<T>() lambda + /// combined with a captured param in the outer Where used to fail generation with + /// QRY037 — AssembledPlan's site-parameter walk skipped the CteDefinition site's + /// inner-param slots, so the outer Where interceptor was emitted against P0 (the + /// CTE's slot) and P1 was never assigned. The inner param must render at slot 0 + /// (WITH clause, rendered first) and the outer param at slot 1. + /// + [Test] + public async Task Cte_FromCte_InnerAndOuterCapturedParams() + { + await using var t = await QueryTestHarness.CreateAsync(); + var (Lite, Pg, My, Ss) = t; + + decimal threshold = 100.00m; + int minId = 2; + + var lt = Lite.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + var pg = Pg.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + var my = My.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + var ss = Ss.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + + QueryTestHarness.AssertDialects( + lt.ToDiagnostics(), pg.ToDiagnostics(), + my.ToDiagnostics(), ss.ToDiagnostics(), + sqlite: "WITH \"Order\" AS (SELECT \"OrderId\", \"UserId\", \"Total\", \"Status\", \"Priority\", \"OrderDate\", \"Notes\" FROM \"orders\" WHERE \"Total\" > @p0) SELECT \"OrderId\", \"Total\" FROM \"Order\" WHERE \"OrderId\" >= @p1", + pg: "WITH \"Order\" AS (SELECT \"OrderId\", \"UserId\", \"Total\", \"Status\", \"Priority\", \"OrderDate\", \"Notes\" FROM \"orders\" WHERE \"Total\" > $1) SELECT \"OrderId\", \"Total\" FROM \"Order\" WHERE \"OrderId\" >= $2", + mysql: "WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `OrderDate`, `Notes` FROM `orders` WHERE `Total` > ?) SELECT `OrderId`, `Total` FROM `Order` WHERE `OrderId` >= ?", + ss: "WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [OrderDate], [Notes] FROM [orders] WHERE [Total] > @p0) SELECT [OrderId], [Total] FROM [Order] WHERE [OrderId] >= @p1"); + + // Seed data: OrderId=1 Total=250.00, OrderId=2 Total=75.50, OrderId=3 Total=150.00 + // CTE keeps orders 1 (250.00) and 3 (150.00); OrderId >= 2 keeps only order 3. + // A slot swap (Total > 2 ... OrderId >= 100) returns empty; a dropped outer + // param (OrderId >= 0) also returns order 1. + var results = await lt.ExecuteFetchAllAsync(); + Assert.That(results, Is.EqualTo(new[] { (3, 150.00m) })); + + var pgResults = await pg.ExecuteFetchAllAsync(); + Assert.That(pgResults, Is.EqualTo(new[] { (3, 150.00m) })); + + var myResults = await my.ExecuteFetchAllAsync(); + Assert.That(myResults, Is.EqualTo(new[] { (3, 150.00m) })); + + var ssResults = await ss.ExecuteFetchAllAsync(); + Assert.That(ssResults, Is.EqualTo(new[] { (3, 150.00m) })); + } + #endregion #region CTE FromCte (identity / all columns) From 6a4a2d5d711ab2f0b7bb0c1a4d124885394a279f Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 18:55:26 -0400 Subject: [PATCH 3/9] Add MySQL execution pin for inner+outer parameterized CTE bind order (#305) Closes the #303 audit gap: the inner+outer combination was the one parameterized-CTE shape whose inner-before-outer positional bind order could not be execution-verified while QRY037 blocked the build. Also retires the QRY037 caveats in the sibling ParameterizedCte* pins. --- .../plan.md | 2 +- .../Integration/MySqlIntegrationTests.cs | 34 ++++++++++++++++--- .../ManifestOutput/quarry-manifest.mysql.md | 17 ++++++++-- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/_sessions/305-fix-cte-outer-param-assignment/plan.md b/_sessions/305-fix-cte-outer-param-assignment/plan.md index 47fc1ac6..10eaa27b 100644 --- a/_sessions/305-fix-cte-outer-param-assignment/plan.md +++ b/_sessions/305-fix-cte-outer-param-assignment/plan.md @@ -86,7 +86,7 @@ static and has no access to `Plan.CteDefinitions`. `(3, 150.00m)`). Run suite. Commit. Tests: the new cross-dialect test itself. -- [ ] **Step 3 — MySQL bind-order execution pin.** +- [x] **Step 3 — MySQL bind-order execution pin.** Add `ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter` to `src/Quarry.Tests/Integration/MySqlIntegrationTests.cs` exactly as specified in issue #305 (companion to the three #304 `ParameterizedCte*` pins; same seed data). Also update diff --git a/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs b/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs index 220974f5..ea410190 100644 --- a/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs +++ b/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs @@ -366,9 +366,8 @@ public async Task ParameterizedCteFilter_OnMySQL_BindsInnerParamsBeforeOuter() // SQL-text order equals chain order (identity) by construction — this pins that // claim with a real MySqlConnector execution (the pre-existing CTE tests inline // literal filters, leaving parameterized CTE chains unexecuted on MySQL). - // The outer filter stays literal: combining a captured inner param with a - // captured outer param trips pre-existing generator defect QRY037 (the outer - // carrier param field is never assigned) — tracked as a separate issue. + // The outer filter stays literal here; the inner+outer combination is pinned + // by ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter (#305). await using var t = await QueryTestHarness.CreateAsync(); var (_, _, My, _) = t; @@ -389,8 +388,7 @@ public async Task ParameterizedCteFilter_OnMySQL_BindsInnerParamsBeforeOuter() [Test] public async Task ParameterizedCteTwoInnerParams_OnMySQL_BindsBothInTextOrder() { - // Two captured params inside the CTE lambda — the maximal multi-param CTE shape - // that builds today (QRY037 blocks only the inner+outer combination). Both + // Two captured params inside the CTE lambda. Both // rebased slots must reach their `?`s in WITH-clause text order. Values are // range-disjoint so a slot swap (Total > 2 AND OrderId >= 100) returns empty // instead of a subtly different set. @@ -434,6 +432,32 @@ public async Task ParameterizedCteOuterParam_OnMySQL_BindsOuterParamAfterLiteral "Expected only order 3 — a different set means minId missed the outer `?`."); } + [Test] + public async Task ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter() + { + // The former QRY037 shape (#305): captured param in the CTE lambda AND a + // captured param in the outer Where. Inner params rebase to the leading slots + // and WITH renders first, so text order is [threshold(0), minId(1)] — identity. + // A slot swap executes WHERE Total > 2 ... OrderId >= 100 and returns empty + // instead of order 3. + await using var t = await QueryTestHarness.CreateAsync(); + var (_, _, My, _) = t; + + decimal threshold = 100.00m; + int minId = 2; + + var rows = await My.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .ExecuteFetchAllAsync(); + + // CTE keeps orders 1 (250.00) and 3 (150.00); OrderId >= 2 keeps order 3. + Assert.That(rows, Is.EqualTo(new[] { (3, 150.00m) }), + "Expected only order 3 — empty means the inner and outer params swapped slots; " + + "a build failure means QRY037 regressed."); + } + [Test] public async Task ParameterizedUnion_OnMySQL_BindsOperandParamsInChainOrder() { diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md index c6c36ff6..99d87287 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md @@ -5109,6 +5109,19 @@ WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `Ord --- +### With(...).FromCte(...).Where(...).Select(...).ExecuteFetchAllAsync() + +```sql +WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `OrderDate`, `Notes` FROM `orders` WHERE `Total` > ?) SELECT `OrderId`, `Total` FROM `Order` WHERE `OrderId` >= ? +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `int` | + +--- + ### With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5241,7 +5254,7 @@ SELECT `UserId`, `UserName`, `Email`, `IsActive`, `CreatedAt`, `LastLogin` FROM | Metric | Count | |--------|------:| -| Total discovered | 574 | +| Total discovered | 575 | | Skipped (errors) | 0 | | Consolidated (deduped) | 99 | -| Rendered | 475 | +| Rendered | 476 | From 661cee3ca82bbecb00c1404583b07db57bb54ffc Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 19:20:48 -0400 Subject: [PATCH 4/9] Address review findings: conditional+CTE QRY048 pin, multi-CTE+outer test, exact-count assertions (#305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F2: ConditionalOuterClause_AfterParameterizedCte_MySQL_NoQRY048 — the only shape that observes BuildParamConditionalMap's CteDefinition offset advance (verified to fail when only that branch is removed). - F3: Cte_TwoChainedWiths_CapturedParams_AndOuterCapturedParam — cumulative offset accumulation across two param-bearing CTEs plus an outer captured param, SQL + execution on all four dialects. - F4: generation guard tightened to exact-count P0/P1 assertions so a wrong-interceptor routing cannot pass. --- .../review.md | 49 +++++++++++++ .../workflow.md | 13 +++- .../Generation/CarrierGenerationTests.cs | 17 +++-- .../MySqlBindOrderGenerationTests.cs | 73 +++++++++++++++++++ .../ManifestOutput/quarry-manifest.mysql.md | 18 ++++- .../quarry-manifest.postgresql.md | 18 ++++- .../ManifestOutput/quarry-manifest.sqlite.md | 18 ++++- .../quarry-manifest.sqlserver.md | 18 ++++- .../SqlOutput/CrossDialectCteTests.cs | 71 ++++++++++++++++++ 9 files changed, 281 insertions(+), 14 deletions(-) create mode 100644 _sessions/305-fix-cte-outer-param-assignment/review.md diff --git a/_sessions/305-fix-cte-outer-param-assignment/review.md b/_sessions/305-fix-cte-outer-param-assignment/review.md new file mode 100644 index 00000000..dec62904 --- /dev/null +++ b/_sessions/305-fix-cte-outer-param-assignment/review.md @@ -0,0 +1,49 @@ +# Review: 305-fix-cte-outer-param-assignment + +## Classifications + +| ID | Class | Rec | Sev | Section | Finding | Action Taken | +|----|-------|-----|-----|---------|---------|--------------| +| F1 | D | D | L | Plan Compliance | Plan said "inline branch"; implementation factored counting into shared `GetCteInnerParamCount` helper — benign, behavior identical | dismissed (benign refinement, noted for the record) | +| F2 | B | B | M | Test Quality | Conditional-map half of fix has no alignment-observing test (conditional outer clause after parameterized CTE on MySQL is newly reachable, untested) | Added ConditionalOuterClause_AfterParameterizedCte_MySQL_NoQRY048 (MySqlBindOrderGenerationTests) — verified it fails when only the conditional-map branch is removed | +| F3 | B | B | L | Test Quality | No multi-CTE (two param-bearing Withs) + outer captured param test — cumulative offset accumulation uncovered | Added Cte_TwoChainedWiths_CapturedParams_AndOuterCapturedParam (CrossDialectCteTests) — 2 param CTEs + outer param, SQL + execution on all 4 dialects | +| F4 | B | B | L | Test Quality | Generation test regexes match anywhere in file; wrong-interceptor assignment would still pass | Tightened CteInnerAndOuterCapturedParams_NoQRY037_AssignsBothPFields to exact-count assertions (1x P0, 1x P1) | +| F5 | C | C | L | Codebase Consistency | `GetCteInnerParamCount` is the 3rd/4th copy of the CteDef-by-short-name first-match loop — consolidation opportunity | issue creation pending user confirmation (outward-facing); to file: consolidate CteDef-by-name lookup into shared helper | + +## Plan Compliance + +| ID | Finding | Sev | Why It Matters | +|----|---------|-----|----------------| +| F1 | Plan specified the CteDefinition advance "inline" in both walks (BuildParamConditionalMap "needs the same branch inline in its loop... because GetClauseParamCount is static"); the implementation instead factors the name-match lookup into a new private instance helper `GetCteInnerParamCount` (AssembledPlan.cs:291) called from an inline branch in each walk. | L | Benign deviation — the branch is still inline per walk and only the counting is shared; behavior is identical to the plan's snippet and duplication is reduced. Noted for the record only. All three plan steps otherwise match exactly: fix + QRY037-region generation test (CarrierGenerationTests.cs:4489, inside the QRY037 region at 4349–4662), the four-dialect Prepare/AssertDialects/execute test with the specified placeholder shapes and `(3, 150.00m)` expectation, the MySQL pin named exactly as planned, and both stale QRY037 comments updated. No scope creep. | + +## Correctness + +No concerns. + +(Verified: CteDefinition maps to a ClauseRole so its entries do appear in `GetClauseEntries()` and the new branches are reachable; branch placement before `Clause != null` is safe because CteDefinition sites always have null Clause and cannot match the set-op or UpdateSetPoco cases; multi-CTE chains accumulate correctly because clause entries walk in chain order and ChainAnalyzer assigns each `CteDef.ParameterOffset` in the same order; zero-inner-param CTEs advance 0 (unchanged behavior); a failed-analysis CTE has no CteDef, `GetCteInnerParamCount` returns 0, and the chain already carries QRY080; duplicate names are first-match, consistent with TransitionBodyEmitter.EmitCteDefinition:199-203 and already rejected by QRY082; `ExtractShortName(null)` returns null which never equals a non-null `CteDef.Name`, so no NRE; `CteEntityTypeName` is always populated for With sites (UsageSiteDiscovery.cs:3784-3787, With is always generic); `RewriteMySqlBindMarkers` (PipelineOrchestrator.cs:641) treats missing conditional-map keys as active, matching the skip-without-entries choice.) + +## Security + +No concerns. + +(The change emits no new code or SQL text; it only realigns integer offsets computed from generator-internal metadata. CTE names derive from C# type identifiers already constrained by the compiler and handled by the pre-existing quoting/emission paths.) + +## Test Quality + +| ID | Finding | Sev | Why It Matters | +|----|---------|-----|----------------| +| F2 | The `BuildParamConditionalMap` half of the fix is only structurally exercised (the branch runs for MySQL CTE chains via `RewriteMySqlBindMarkers`), but no test observes its key *alignment*: with only unconditional clauses, every param is treated as active regardless of map keys, so a misaligned offset is invisible. The shape that would observe it — a conditional outer clause (`if (x) q = q.Where(...)`) after a parameterized CTE on MySQL — was previously unreachable (QRY037) and is now newly buildable, yet remains untested. | M | A future regression in this walk would silently mislabel slots and surface only as a hard-to-diagnose QRY048/identity-bind failure (or wrong active-set validation) for conditional+CTE chains on MySQL. The plan's own working notes flagged this combination as "must be fixed consistently"; a pin for the newly reachable shape would close the loop. | +| F3 | No test covers a multi-CTE chain (two `With` with inner params each) combined with an outer captured param — the shape that exercises *cumulative* offset accumulation across multiple CteDefinition entries. Existing `LambdaCte_TwoChainedWiths_CapturedParams` (LambdaCteTests.cs:104) has no outer param; the new tests cover a single CTE only. | L | The per-entry advance makes cumulative correctness nearly self-evident, but multi-CTE was listed as a design boundary condition and this newly-enabled shape (three global slots: two inner + one outer) has zero coverage on any dialect. | +| F4 | The generation test's regex assertions (`__c\.P0\s*=`, `__c\.P1\s*=`) match anywhere in the interceptor file, not scoped to the With/Where interceptor bodies respectively, and are largely implied by the QRY037-absence assertion (the self-check already guarantees every declared P-field is assigned somewhere). A regression that assigns P1 from the wrong interceptor would pass this test. | L | Low residual risk in practice — the cross-dialect and MySQL execution tests catch slot-value swaps at runtime — but the generation test's stated intent ("the outer Where interceptor must assign... not stomp P0") is stronger than what it asserts. | + +## Codebase Consistency + +| ID | Finding | Sev | Why It Matters | +|----|---------|-----|----------------| +| F5 | `GetCteInnerParamCount` is the third/fourth copy of the "resolve a CteDefinition site to its CteDef by `ExtractShortName(CteEntityTypeName ?? EntityTypeName)` first-match" loop — the same pattern exists in CarrierAnalyzer.BuildLambdaInnerExtractionPlan (CarrierAnalyzer.cs:456-462) and twice in TransitionBodyEmitter.EmitCteDefinition (TransitionBodyEmitter.cs:173-221). | L | The duplication faithfully follows the established repo pattern (and the fix's comments cross-reference it), but a shared `FindCteDef(site, cteDefinitions)` helper next to `CteNameHelpers` would give the "MUST use the same helper" doc contract in CteDef.cs:121-128 a single enforcement point. Purely a consolidation opportunity, not a defect. Otherwise the change is idiomatic: the new branch mirrors the adjacent `setOpIndex` handling shape, comment style/density matches the file, and the conditional-map branch uses the same `continue` pattern as the set-op case. | + +## Integration / Breaking Changes + +No concerns. + +(The change is purely additive in effect: the new branch fires only for CteDefinition entries, and for every previously-buildable chain shape the computed offsets are unchanged — chains with zero-inner-param CTEs advance by 0 as before, and all shapes where the offset would differ were hard build errors (QRY037) on master. Manifest golden churn is exactly the expected additions — one new `With(...).FromCte(...).Where(...).Select(...)` Prepare entry per dialect from CrossDialectCteTests plus one MySQL ExecuteFetchAllAsync entry from the integration pin, with counts advancing accordingly (+2 MySQL, +1 elsewhere) and no modifications to any existing rendered SQL. No public API surface changed.) diff --git a/_sessions/305-fix-cte-outer-param-assignment/workflow.md b/_sessions/305-fix-cte-outer-param-assignment/workflow.md index b6cdaf73..80e080b2 100644 --- a/_sessions/305-fix-cte-outer-param-assignment/workflow.md +++ b/_sessions/305-fix-cte-outer-param-assignment/workflow.md @@ -5,7 +5,7 @@ platform: github base-branch: master ## State -phase: IMPLEMENT +phase: REVIEW status: active issue: #305 pr: @@ -51,6 +51,10 @@ Baseline test status: all green at 7bb0e35 — Quarry.Tests 3281 passed, Quarry. `ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter` exactly as specified in issue #305. +- 2026-07-02: REVIEW classifications applied as recommended (user AFK at prompt; same + autonomous-default as DESIGN): F1→D, F2→B, F3→B, F4→B, F5→C. Final: 0A/3B/1C/1D. + F5's issue creation deferred until user confirmation (outward-facing action). + ## Working Notes - **Root cause (confirmed by code trace, DESIGN 2026-07-02):** not in ChainAnalyzer or SqlAssembler — both are correct. `ChainAnalyzer` inserts CTE inner params into the @@ -83,6 +87,13 @@ Baseline test status: all green at 7bb0e35 — Quarry.Tests 3281 passed, Quarry. AssembledPlan covers every consumer. - Generator llm.md's QRY table skips QRY037 (jumps QRY036→QRY040) — pre-existing doc gap, out of scope here. +- **Gotcha (REMEDIATE):** restoring a source file via `mv backup.cs file.cs` preserves the + backup's OLD mtime — msbuild then treats the project as up-to-date and keeps the stale + assembly built from the intermediate (mutated) source. Symptom here: the F2 QRY048 pin + passed, then failed after the bite-verification cycle, because the generator DLL still + had the conditional-map branch removed. Fix: `touch` the file (or `git checkout --`, + which sets a fresh mtime) before rebuilding. When bite-verifying by mutating source, + prefer `git stash` / `git checkout -- ` over manual backup files. - Main repo (Quarry-master) has an uncommitted, unrelated change to `src/Quarry.Generator/CodeGen/CarrierEmitter.cs` (Patch-chain `__colShift` CS0219 avoidance). It is NOT part of this branch; worktree branched from clean diff --git a/src/Quarry.Tests/Generation/CarrierGenerationTests.cs b/src/Quarry.Tests/Generation/CarrierGenerationTests.cs index b883b663..a2e6f14c 100644 --- a/src/Quarry.Tests/Generation/CarrierGenerationTests.cs +++ b/src/Quarry.Tests/Generation/CarrierGenerationTests.cs @@ -4529,11 +4529,18 @@ await db.With(orders => orders.Where(o => o.Total > threshold)) Assert.That(tree, Is.Not.Null, "Interceptor file must be generated for the CTE chain"); var code = tree!.GetText().ToString(); - Assert.That(code, Does.Match(@"__c\.P0\s*="), - "The With interceptor must assign the CTE's inner captured param to P0"); - Assert.That(code, Does.Match(@"__c\.P1\s*="), - "The outer Where interceptor must assign its captured param to P1 (the " - + "slot after the CTE's inner param), not stomp P0"); + // Count-based: the pre-fix stomp signature was TWO P0 assignments (CTE capture + // in the With interceptor + the misrouted outer Where) and ZERO P1 assignments. + // The QRY037-absence assertion above already implies P1 is assigned somewhere; + // the exact counts additionally reject a wrong-interceptor routing where P1 is + // assigned but P0 is still double-written. + var p0Assignments = System.Text.RegularExpressions.Regex.Matches(code, @"__c\.P0\s*=").Count; + var p1Assignments = System.Text.RegularExpressions.Regex.Matches(code, @"__c\.P1\s*=").Count; + Assert.That(p0Assignments, Is.EqualTo(1), + "Exactly one P0 assignment expected (the With interceptor's CTE capture); " + + "two means the outer Where is stomping the CTE's slot again"); + Assert.That(p1Assignments, Is.EqualTo(1), + "Exactly one P1 assignment expected (the outer Where interceptor)"); } /// diff --git a/src/Quarry.Tests/Generation/MySqlBindOrderGenerationTests.cs b/src/Quarry.Tests/Generation/MySqlBindOrderGenerationTests.cs index b1f0d04e..f3d7fe77 100644 --- a/src/Quarry.Tests/Generation/MySqlBindOrderGenerationTests.cs +++ b/src/Quarry.Tests/Generation/MySqlBindOrderGenerationTests.cs @@ -208,6 +208,79 @@ public void TwoIndependentConditionalFilters_WrapChain_MySQL_NoQRY048_BindsHoist Assert.That(p1, Is.LessThan(p2), "WHERE slots keep clause order after the hoisted slot"); } + private const string ConditionalAfterParameterizedCteSource = @" +using Quarry; +namespace TestApp; + +public class OrderSchema : Schema +{ + public static string Table => ""orders""; + public Key OrderId => Identity(); + public Col UserId { get; } + public Col Total { get; } +} + +[QuarryContext(Dialect = SqlDialect.MySQL)] +public partial class TestDbContext : QuarryContext +{ + public partial IEntityAccessor Orders(); +} + +public static class Queries +{ + public static async Task Test(TestDbContext db, bool byMax) + { + decimal threshold = 100.00m; + int minId = 1; + int maxId = 3; + IQueryBuilder q = db.With(orders => orders.Where(o => o.Total > threshold)) + .FromCte() + .Where(o => o.OrderId >= minId); + if (byMax) { q = q.Where(o => o.OrderId <= maxId); } + var rows = await q + .Select(o => (o.OrderId, o.Total)) + .ExecuteFetchAllAsync(); + } +} +"; + + [Test] + public void ConditionalOuterClause_AfterParameterizedCte_MySQL_NoQRY048() + { + // Issue #305 remediation (review F2): the alignment-observing pin for + // AssembledPlan.BuildParamConditionalMap's CteDefinition offset advance. + // Slots: threshold(0, CTE inner), minId(1, unconditional outer), + // maxId(2, conditional bit 0). RewriteMySqlBindMarkers validates each SQL + // variant's placeholder slot set against the conditional map's expected + // active set — if the CTE's inner-param slot were skipped in that walk, the + // conditional flag would land on slot 1 instead of slot 2, the mask-0 + // variant's active set would exclude a slot its text contains, and the + // validation would fail loudly with QRY048. Unconditional chains cannot + // observe this (every param is active in every variant regardless of keys), + // so this previously-QRY037-blocked shape is the only pin for the + // conditional-map half of the #305 fix. + var (code, diagnostics) = RunGenerator(ConditionalAfterParameterizedCteSource); + + Assert.That(diagnostics.Where(d => d.Id == "QRY037"), Is.Empty, + "Inner+outer captured CTE params must build (#305)"); + Assert.That(diagnostics.Where(d => d.Id == "QRY048"), Is.Empty, + "A QRY048 here means the conditional map's keys are misaligned with the " + + "chain's parameter slots — the CteDefinition offset advance regressed"); + Assert.That(code, Does.Not.Contain("{__Q"), + "Bind-order markers must never leak into generated source"); + + // WITH renders first and no wrap/hoist applies, so text order is identity; + // the bind blocks must stay in slot order. + var p0 = code.IndexOf("var __p0 = __cmd.CreateParameter();", StringComparison.Ordinal); + var p1 = code.IndexOf("var __p1 = __cmd.CreateParameter();", StringComparison.Ordinal); + var p2 = code.IndexOf("var __p2 = __cmd.CreateParameter();", StringComparison.Ordinal); + Assert.That(p0, Is.GreaterThanOrEqualTo(0), "P0 (CTE inner) bind block should exist"); + Assert.That(p1, Is.GreaterThanOrEqualTo(0), "P1 (outer Where) bind block should exist"); + Assert.That(p2, Is.GreaterThanOrEqualTo(0), "P2 (conditional Where) bind block should exist"); + Assert.That(p0, Is.LessThan(p1), "CTE inner param binds before the outer param (WITH renders first)"); + Assert.That(p1, Is.LessThan(p2), "Unconditional outer param binds before the conditional one"); + } + private const string MarkerLiteralSource = @" using Quarry; namespace TestApp; diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md index 99d87287..77c1c70d 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.mysql.md @@ -5160,6 +5160,20 @@ WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `Ord --- +### With(...).With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `OrderDate`, `Notes` FROM `orders` WHERE `Total` > ?), `User` AS (SELECT `UserId`, `UserName`, `Email`, `IsActive`, `CreatedAt`, `LastLogin` FROM `users` WHERE `IsActive` = ?) SELECT `OrderId`, `Total` FROM `Order` WHERE `OrderId` >= ? +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `bool` | +| `@p2` | `int` | + +--- + ### With(...).With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5254,7 +5268,7 @@ SELECT `UserId`, `UserName`, `Email`, `IsActive`, `CreatedAt`, `LastLogin` FROM | Metric | Count | |--------|------:| -| Total discovered | 575 | +| Total discovered | 576 | | Skipped (errors) | 0 | | Consolidated (deduped) | 99 | -| Rendered | 476 | +| Rendered | 477 | diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md index 9304f3e5..0a7e469a 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.postgresql.md @@ -5267,6 +5267,20 @@ WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "Ord --- +### With(...).With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "OrderDate", "Notes" FROM "orders" WHERE "Total" > $1), "User" AS (SELECT "UserId", "UserName", "Email", "IsActive", "CreatedAt", "LastLogin" FROM "users" WHERE "IsActive" = $2) SELECT "OrderId", "Total" FROM "Order" WHERE "OrderId" >= $3 +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `bool` | +| `@p2` | `int` | + +--- + ### With(...).With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5293,7 +5307,7 @@ SELECT "UserId", "UserName", "Email", "IsActive", "CreatedAt", "LastLogin" FROM | Metric | Count | |--------|------:| -| Total discovered | 602 | +| Total discovered | 603 | | Skipped (errors) | 0 | | Consolidated (deduped) | 117 | -| Rendered | 485 | +| Rendered | 486 | diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md index 43646a1d..f2374c1f 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlite.md @@ -5614,6 +5614,20 @@ WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "Ord --- +### With(...).With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "OrderDate", "Notes" FROM "orders" WHERE "Total" > @p0), "User" AS (SELECT "UserId", "UserName", "Email", "IsActive", "CreatedAt", "LastLogin" FROM "users" WHERE "IsActive" = @p1) SELECT "OrderId", "Total" FROM "Order" WHERE "OrderId" >= @p2 +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `bool` | +| `@p2` | `int` | + +--- + ### With(...).With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5632,7 +5646,7 @@ WITH "Order" AS (SELECT "OrderId", "UserId", "Total", "Status", "Priority", "Ord | Metric | Count | |--------|------:| -| Total discovered | 707 | +| Total discovered | 708 | | Skipped (errors) | 0 | | Consolidated (deduped) | 193 | -| Rendered | 514 | +| Rendered | 515 | diff --git a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md index b73a9f4a..09c3ba76 100644 --- a/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md +++ b/src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md @@ -5087,6 +5087,20 @@ WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [Ord --- +### With(...).With(...).FromCte(...).Where(...).Select(...).Prepare().ToDiagnostics() + +```sql +WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [OrderDate], [Notes] FROM [orders] WHERE [Total] > @p0), [User] AS (SELECT [UserId], [UserName], [Email], [IsActive], [CreatedAt], [LastLogin] FROM [users] WHERE [IsActive] = @p1) SELECT [OrderId], [Total] FROM [Order] WHERE [OrderId] >= @p2 +``` + +| Parameter | Type | +|-----------|------| +| `@p0` | `decimal` | +| `@p1` | `bool` | +| `@p2` | `int` | + +--- + ### With(...).With(...).With(...).FromCte(...).Select(...).Prepare().ToDiagnostics() ```sql @@ -5105,7 +5119,7 @@ WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [Ord | Metric | Count | |--------|------:| -| Total discovered | 567 | +| Total discovered | 568 | | Skipped (errors) | 0 | | Consolidated (deduped) | 101 | -| Rendered | 466 | +| Rendered | 467 | diff --git a/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs b/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs index b13a93d2..4adbcc2e 100644 --- a/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs +++ b/src/Quarry.Tests/SqlOutput/CrossDialectCteTests.cs @@ -238,6 +238,77 @@ public async Task Cte_FromCte_InnerAndOuterCapturedParams() Assert.That(ssResults, Is.EqualTo(new[] { (3, 150.00m) })); } + /// + /// Issue #305 remediation (review F3): cumulative offset accumulation across + /// MULTIPLE param-bearing CteDefinition entries combined with an outer captured + /// param. Two CTEs contribute slots 0 and 1; the outer Where must land at slot 2 — + /// a per-entry advance that only accumulates the first CTE would emit the outer + /// assignment against slot 1 (the second CTE's) and trip QRY037 or misbind. + /// + [Test] + public async Task Cte_TwoChainedWiths_CapturedParams_AndOuterCapturedParam() + { + await using var t = await QueryTestHarness.CreateAsync(); + var (Lite, Pg, My, Ss) = t; + + decimal orderCutoff = 100m; + bool activeFilter = true; + int minId = 2; + + var lt = Lite + .With(orders => orders.Where(o => o.Total > orderCutoff)) + .With(users => users.Where(u => u.IsActive == activeFilter)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + var pg = Pg + .With(orders => orders.Where(o => o.Total > orderCutoff)) + .With(users => users.Where(u => u.IsActive == activeFilter)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + var my = My + .With(orders => orders.Where(o => o.Total > orderCutoff)) + .With(users => users.Where(u => u.IsActive == activeFilter)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + var ss = Ss + .With(orders => orders.Where(o => o.Total > orderCutoff)) + .With(users => users.Where(u => u.IsActive == activeFilter)) + .FromCte() + .Where(o => o.OrderId >= minId) + .Select(o => (o.OrderId, o.Total)) + .Prepare(); + + QueryTestHarness.AssertDialects( + lt.ToDiagnostics(), pg.ToDiagnostics(), + my.ToDiagnostics(), ss.ToDiagnostics(), + sqlite: "WITH \"Order\" AS (SELECT \"OrderId\", \"UserId\", \"Total\", \"Status\", \"Priority\", \"OrderDate\", \"Notes\" FROM \"orders\" WHERE \"Total\" > @p0), \"User\" AS (SELECT \"UserId\", \"UserName\", \"Email\", \"IsActive\", \"CreatedAt\", \"LastLogin\" FROM \"users\" WHERE \"IsActive\" = @p1) SELECT \"OrderId\", \"Total\" FROM \"Order\" WHERE \"OrderId\" >= @p2", + pg: "WITH \"Order\" AS (SELECT \"OrderId\", \"UserId\", \"Total\", \"Status\", \"Priority\", \"OrderDate\", \"Notes\" FROM \"orders\" WHERE \"Total\" > $1), \"User\" AS (SELECT \"UserId\", \"UserName\", \"Email\", \"IsActive\", \"CreatedAt\", \"LastLogin\" FROM \"users\" WHERE \"IsActive\" = $2) SELECT \"OrderId\", \"Total\" FROM \"Order\" WHERE \"OrderId\" >= $3", + mysql: "WITH `Order` AS (SELECT `OrderId`, `UserId`, `Total`, `Status`, `Priority`, `OrderDate`, `Notes` FROM `orders` WHERE `Total` > ?), `User` AS (SELECT `UserId`, `UserName`, `Email`, `IsActive`, `CreatedAt`, `LastLogin` FROM `users` WHERE `IsActive` = ?) SELECT `OrderId`, `Total` FROM `Order` WHERE `OrderId` >= ?", + ss: "WITH [Order] AS (SELECT [OrderId], [UserId], [Total], [Status], [Priority], [OrderDate], [Notes] FROM [orders] WHERE [Total] > @p0), [User] AS (SELECT [UserId], [UserName], [Email], [IsActive], [CreatedAt], [LastLogin] FROM [users] WHERE [IsActive] = @p1) SELECT [OrderId], [Total] FROM [Order] WHERE [OrderId] >= @p2"); + + // Order CTE keeps orders 1 (250.00) and 3 (150.00); OrderId >= 2 keeps order 3. + // A slot skew that binds minId(2) into slot 1 flips the User CTE's IsActive + // comparand instead, and the outer filter binds default(int) = 0 — returning + // order 1 as well. + var results = await lt.ExecuteFetchAllAsync(); + Assert.That(results, Is.EqualTo(new[] { (3, 150.00m) })); + + var pgResults = await pg.ExecuteFetchAllAsync(); + Assert.That(pgResults, Is.EqualTo(new[] { (3, 150.00m) })); + + var myResults = await my.ExecuteFetchAllAsync(); + Assert.That(myResults, Is.EqualTo(new[] { (3, 150.00m) })); + + var ssResults = await ss.ExecuteFetchAllAsync(); + Assert.That(ssResults, Is.EqualTo(new[] { (3, 150.00m) })); + } + #endregion #region CTE FromCte (identity / all columns) From 9341cb8aa3c98ca67bbbfc22b9b4a85384c7c951 Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 19:21:29 -0400 Subject: [PATCH 5/9] chore: session artifacts (PR body) --- .../_pr-body.md | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 _sessions/305-fix-cte-outer-param-assignment/_pr-body.md diff --git a/_sessions/305-fix-cte-outer-param-assignment/_pr-body.md b/_sessions/305-fix-cte-outer-param-assignment/_pr-body.md new file mode 100644 index 00000000..d77be618 --- /dev/null +++ b/_sessions/305-fix-cte-outer-param-assignment/_pr-body.md @@ -0,0 +1,38 @@ +## Summary +- Closes #305 +- Fixes the QRY037 build failure for CTE chains that combine a captured parameter inside the `With(...)` lambda with a captured parameter in an outer clause (`Where` after `FromCte()`). + +## Reason for Change +The inner+outer parameterized CTE shape — valid SQL on all four dialects — was unbuildable: the generated carrier declared the outer parameter's field (`P1`) but no clause interceptor assigned it, tripping the generator's QRY037 self-check. + +**Root cause** (differs from the issue's suspicion): `ChainAnalyzer`'s parameter rebasing and `SqlAssembler`'s placeholder rebasing are both correct. The defect was in `AssembledPlan.BuildSiteParamsMap` / `BuildParamConditionalMap`: both walk clause entries accumulating the global parameter offset each emitter uses to route `__c.P{n}` assignments, but a `CteDefinition` entry (whose `Clause` is null — it is not clause-bearing) advanced the offset by 0 despite owning `InnerParameters.Count` slots in the chain's parameter list. Every param-bearing clause after a param-bearing `With()` therefore resolved its carrier P-fields short by the inner-param count: the outer `Where` wrote `P0` (the CTE's slot) and `P1` was never assigned. With zero inner params the skew is zero — exactly why inner-only, two-inner, and outer-only chains all worked. + +**Fix**: advance the offset by the matching `CteDef.InnerParameters.Count`, matched by CTE short name via `CteNameHelpers.ExtractShortName` (the same first-match rule as `TransitionBodyEmitter.EmitCteDefinition`; duplicate names are already QRY082 errors, failed-analysis CTEs already QRY080). All emitter paths funnel through this one map, so the single fix point covers every consumer. + +## Impact +- Inner+outer parameterized CTE chains (single- and multi-CTE) now build and bind correctly on all four dialects. +- No behavior change for any previously-buildable chain: the new branch fires only for `CteDefinition` entries, and every shape whose offsets would differ was a hard QRY037 build error before. +- Also closes the #303 audit gap: the inner+outer combination was the one parameterized-CTE shape whose MySQL inner-before-outer positional bind order could not be execution-verified. + +## Plan items implemented as specified +1. `AssembledPlan` offset-walk fix + generation guard (`CteInnerAndOuterCapturedParams_NoQRY037_AssignsBothPFields`) — verified to fail without the fix. +2. Cross-dialect SQL + execution test (`Cte_FromCte_InnerAndOuterCapturedParams`) — exact WITH rendering and row verification on all four backends. +3. MySQL bind-order execution pin (`ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter`), exactly as specified in #305, plus retirement of the QRY037 caveats in the sibling `ParameterizedCte*` pins. + +## Deviations from plan implemented +- The planned "inline branch" in both walks was refined into a shared `GetCteInnerParamCount` helper called from an inline branch in each walk (review F1, dismissed as benign — behavior identical, duplication reduced). + +## Gaps in original plan implemented +Review findings addressed post-implementation: +- **F2 (M)**: `ConditionalOuterClause_AfterParameterizedCte_MySQL_NoQRY048` — the only shape that observes the `BuildParamConditionalMap` half of the fix (a conditional outer clause after a parameterized CTE on MySQL, newly reachable). Verified to fail when only that branch is removed. +- **F3 (L)**: `Cte_TwoChainedWiths_CapturedParams_AndOuterCapturedParam` — cumulative offset accumulation across two param-bearing CTEs plus an outer captured param, on all four dialects. +- **F4 (L)**: generation guard tightened to exact-count P0/P1 assertions (the pre-fix stomp signature was two P0 assignments and zero P1). + +Deferred (review F5, class C, pending confirmation): consolidating the CteDef-by-short-name first-match lookup (now present in `AssembledPlan`, `CarrierAnalyzer`, and twice in `TransitionBodyEmitter`) into a single shared helper — a cleanup, not a defect. + +## Performance Considerations +The added lookup is a short linear scan over `Plan.CteDefinitions` (almost always 0–3 entries) inside two already-cached map builders — no measurable generator cost. + +## Breaking Changes +- Consumer-facing: none (previously-failing shapes now build; no existing SQL or binding changes — manifest goldens show additions only). +- Internal: none (private helpers only; no signature changes). From 226825492b053286a3f1f56b03d3f602fe643d2a Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 19:21:55 -0400 Subject: [PATCH 6/9] chore: session artifacts (REMEDIATE state) --- _sessions/305-fix-cte-outer-param-assignment/workflow.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/_sessions/305-fix-cte-outer-param-assignment/workflow.md b/_sessions/305-fix-cte-outer-param-assignment/workflow.md index 80e080b2..1d7dca78 100644 --- a/_sessions/305-fix-cte-outer-param-assignment/workflow.md +++ b/_sessions/305-fix-cte-outer-param-assignment/workflow.md @@ -5,10 +5,10 @@ platform: github base-branch: master ## State -phase: REVIEW +phase: REMEDIATE status: active issue: #305 -pr: +pr: #306 ## Problem Statement QRY037 build failure for CTE chains that combine a captured parameter inside the @@ -106,3 +106,4 @@ Baseline test status: all green at 7bb0e35 — Quarry.Tests 3281 passed, Quarry. | Date | Phases | Summary | |------|--------|---------| | 2026-07-02 | INTAKE, DESIGN, PLAN | Loaded issue #305, created worktree + branch, baseline all green (3628 tests). Traced root cause to AssembledPlan offset walk. Design+plan provisionally approved (user AFK); wrote plan.md; entered IMPLEMENT. | +| 2026-07-02 | IMPLEMENT, REVIEW, REMEDIATE | 3 plan steps committed, suite green each step. Review: 5 findings (0A/3B/1C/1D), B-items fixed and bite-verified, F5 issue deferred pending user confirm. PR #306 created; awaiting CI. | From 130e42f835e39dc0327066d18a5c50c5574bc61f Mon Sep 17 00:00:00 2001 From: djgosnell Date: Thu, 2 Jul 2026 19:25:44 -0400 Subject: [PATCH 7/9] chore: session artifacts (FINALIZE state, F5 issue draft) --- .../_issue-f5.md | 21 +++++++++++++++++++ .../workflow.md | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 _sessions/305-fix-cte-outer-param-assignment/_issue-f5.md diff --git a/_sessions/305-fix-cte-outer-param-assignment/_issue-f5.md b/_sessions/305-fix-cte-outer-param-assignment/_issue-f5.md new file mode 100644 index 00000000..70447620 --- /dev/null +++ b/_sessions/305-fix-cte-outer-param-assignment/_issue-f5.md @@ -0,0 +1,21 @@ +## Description +The "resolve a CteDefinition site to its CteDef" lookup — `CteNameHelpers.ExtractShortName(site.Bound.Raw.CteEntityTypeName ?? site.EntityTypeName)` compared against `CteDef.Name`, first match wins — is now duplicated in four places. `CteDef.cs`'s doc contract ("both sides MUST use the same helper so the names compare equal under all input forms") is enforced only by convention; a shared lookup helper would give it a single enforcement point. + +## Location +- `src/Quarry.Generator/IR/AssembledPlan.cs` — `GetCteInnerParamCount` (added in #306) +- `src/Quarry.Generator/CodeGen/CarrierAnalyzer.cs` — `BuildLambdaInnerExtractionPlan` (~line 456) +- `src/Quarry.Generator/CodeGen/TransitionBodyEmitter.cs` — `EmitCteDefinition`, twice (~lines 173–221) +- Contract doc: `src/Quarry.Generator/IR/CteDef.cs` (`CteNameHelpers` remarks, ~lines 121–128) + +## Diagnostics +None — this is a consolidation/maintainability cleanup, not a defect. Flagged as review finding F5 (severity L) during #306's review pass. + +## What Has Been Tried +Nothing yet; the duplication was deliberately kept in #306 to mirror the established pattern rather than widen that PR's scope. + +## Gathered Information +- All four sites use identical semantics: short-name match, first match wins. +- Duplicate CTE names are rejected at compile time by QRY082, and failed-analysis CTEs produce no CteDef (chain already carries QRY080), so first-match/no-match semantics are uniform across call sites. + +## Suggested Approach +Add a static helper next to `CteNameHelpers` (e.g. `CteNameHelpers.FindCteDef(TranslatedCallSite site, IReadOnlyList cteDefinitions)` or an overload taking the raw type name) and route all four call sites through it. Pure refactor — behavior must be byte-identical; the existing CTE test suite (CrossDialectCteTests, LambdaCteTests, MySqlBindOrderGenerationTests, CarrierGenerationTests CTE regions) is the regression gate. diff --git a/_sessions/305-fix-cte-outer-param-assignment/workflow.md b/_sessions/305-fix-cte-outer-param-assignment/workflow.md index 1d7dca78..78ee2ed0 100644 --- a/_sessions/305-fix-cte-outer-param-assignment/workflow.md +++ b/_sessions/305-fix-cte-outer-param-assignment/workflow.md @@ -5,7 +5,7 @@ platform: github base-branch: master ## State -phase: REMEDIATE +phase: FINALIZE status: active issue: #305 pr: #306 From c9d3223c95e2ec8eda323d92ab377b77f89c5f82 Mon Sep 17 00:00:00 2001 From: djgosnell Date: Fri, 3 Jul 2026 22:15:47 -0400 Subject: [PATCH 8/9] Retire remaining stale QRY037-boundary comment in ParameterizedCteOuterParam pin (#305) --- src/Quarry.Tests/Integration/MySqlIntegrationTests.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs b/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs index ea410190..6c5035ca 100644 --- a/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs +++ b/src/Quarry.Tests/Integration/MySqlIntegrationTests.cs @@ -412,10 +412,11 @@ public async Task ParameterizedCteTwoInnerParams_OnMySQL_BindsBothInTextOrder() [Test] public async Task ParameterizedCteOuterParam_OnMySQL_BindsOuterParamAfterLiteralCte() { - // Outer captured param with a literal-filtered CTE — the other side of the - // QRY037 boundary (the outer clause-interceptor assignment works when the CTE - // contributes no params). Pins that the outer `?` receives its value on real - // MySqlConnector with a WITH clause present in the text. + // Outer captured param with a literal-filtered CTE — isolates the outer + // clause-interceptor assignment with a zero-param CTE (rebase offset 0). + // Pins that the outer `?` receives its value on real MySqlConnector with a + // WITH clause present in the text. The inner+outer combination is pinned by + // ParameterizedCteInnerAndOuterParams_OnMySQL_BindsInnerBeforeOuter (#305). await using var t = await QueryTestHarness.CreateAsync(); var (_, _, My, _) = t; From 0bc86e5f0f19f742d11642775432b8f04772b9d7 Mon Sep 17 00:00:00 2001 From: djgosnell Date: Mon, 13 Jul 2026 10:21:17 -0400 Subject: [PATCH 9/9] chore: suspend workflow at FINALIZE (starting #307 in parallel) --- .../305-fix-cte-outer-param-assignment/workflow.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/_sessions/305-fix-cte-outer-param-assignment/workflow.md b/_sessions/305-fix-cte-outer-param-assignment/workflow.md index 78ee2ed0..321e1a5d 100644 --- a/_sessions/305-fix-cte-outer-param-assignment/workflow.md +++ b/_sessions/305-fix-cte-outer-param-assignment/workflow.md @@ -6,7 +6,7 @@ base-branch: master ## State phase: FINALIZE -status: active +status: suspended issue: #305 pr: #306 @@ -101,9 +101,18 @@ Baseline test status: all green at 7bb0e35 — Quarry.Tests 3281 passed, Quarry. also touches CarrierEmitter. ## Suspend State +- Phase: FINALIZE, step 1 (merge-option prompt not yet asked). +- PR #306 open, CI green (build SUCCESS), mergeStateStatus CLEAN as of 2026-07-13. +- In progress: nothing — awaiting user's FINALIZE choice (squash merge / rebase / back to REVIEW). +- Immediate next step: ask FINALIZE step-1 question, then pre-merge cleanup (delete `_sessions/305-fix-cte-outer-param-assignment/`, commit, push) before squash merge. +- No WIP commit; working tree clean. +- Test status: all passing as of last REMEDIATE run. +- Unrecorded context: F5 (class C) issue creation was deferred pending user confirmation — still outstanding; confirm before or during FINALIZE. +- Suspended 2026-07-13 because user chose to start issue #307 in parallel first. ## Session Log | Date | Phases | Summary | |------|--------|---------| +| 2026-07-13 | FINALIZE (suspended) | PR #306 CI green, ready to merge. User chose to start issue #307 in parallel; workflow suspended at FINALIZE step 1. | | 2026-07-02 | INTAKE, DESIGN, PLAN | Loaded issue #305, created worktree + branch, baseline all green (3628 tests). Traced root cause to AssembledPlan offset walk. Design+plan provisionally approved (user AFK); wrote plan.md; entered IMPLEMENT. | | 2026-07-02 | IMPLEMENT, REVIEW, REMEDIATE | 3 plan steps committed, suite green each step. Review: 5 findings (0A/3B/1C/1D), B-items fixed and bite-verified, F5 issue deferred pending user confirm. PR #306 created; awaiting CI. |