---
phase: 65.1-phase-65-code-review-remediation
plan: 05
subsystem: testing
tags: [python, threading, concurrency, integration-test, pep-723, race-conditions, ddl, tech-debt-23]

# Dependency graph
requires:
  - phase: 65-overridecontext-connection-teardown
    provides: "parser_override CREATE/DROP/ALTER on caller's connection (race-guard SQL + json_merge_patch UPDATE); test_concurrent_reads_per_call_conn.py reads-side template"
  - phase: 65.1-phase-65-code-review-remediation
    provides: "65.1-CONTEXT.md D-19/D-20 (WR-04 write-side concurrency coverage with PK-violation tolerance per TECH-DEBT 23)"
provides:
  - "test/integration/test_concurrent_writes_per_call_conn.py — 8 threads × 10 mixed CREATE/DROP/ALTER ops on overlapping view names v_0..v_3"
  - "Five-bucket outcome categorisation: success | already_exists | constraint_violation | tuple_conflict | unknown_error (assert zero unknown_error)"
  - "Final-state subset invariant: list_semantic_views() ⊆ union(success-marked CREATEs) — catches spurious rows without over-pinning race ordering"
  - "Documented second PK-violation wording (commit-time TransactionContext shape) alongside the TECH-DEBT 23 single-statement check-time wording"
  - "Documented DuckDB optimistic-concurrency tuple-deletion conflict as the upstream serialisation contract for racing DROPs/ALTERs on the same _definitions row"
affects: [65.1-09, milestone-v0.10.0-release-notes]

# Tech tracking
tech-stack:
  added: []
  patterns:
    - "PEP-723 + open_master fixture + threading.Event start-gate (verbatim from test_concurrent_reads_per_call_conn.py)"
    - "Per-thread WorkerResult dataclass with per-thread success ledgers (create_successes / drop_successes) folded across threads at assert time"
    - "Subset-of-success-CREATEs final-state invariant (NOT exact-equality minus DROPs — race ordering forbids strict equality per Pitfall 4 / Assumption A4)"

key-files:
  created:
    - "test/integration/test_concurrent_writes_per_call_conn.py — 340 LOC; mirrors reads template shape; ALTER uses IF EXISTS so DROP-then-ALTER races are silent no-ops"
  modified: []

key-decisions:
  - "ALTER uses IF EXISTS — a thread issuing ALTER on a view that was just DROPped by another thread becomes a silent no-op rather than a 'does not exist' error, matching realistic bootstrap-retry script shapes and keeping the test's no-unknown-errors contract clean"
  - "Five-bucket categorisation (one more than the plan's four-bucket plan): added tuple_conflict for DuckDB optimistic-concurrency rollback ('Conflict on tuple deletion!'). Discovered empirically during execution — DuckDB serialises concurrent UPDATEs/DELETEs against the same _definitions row at commit time, which is documented upstream behaviour. Treating it as a known race rather than unknown_error keeps the test's contract aligned with the underlying engine's actual concurrency semantics"
  - "Second PK-violation wording captured: the commit-time TransactionContext path ('Failed to commit: PRIMARY KEY or UNIQUE constraint violation: duplicate key X') is distinct from TECH-DEBT 23's documented single-statement check-time path ('Constraint Error: Duplicate key X violates primary key constraint'). Both shapes categorise as constraint_violation"
  - "Subset invariant over exact equality: race ordering means a thread's CREATE may be followed by another thread's DROP and vice versa, so final_views must merely be a subset of union(success-CREATEs). This is the robust invariant per researcher's Pitfall 4"
  - "INSERT VALUES added to SETUP_SQL so count(*) over t1 produces a non-trivial cardinality for any post-test sanity checks (no functional dependency for the test itself, but reflects the reads-template style)"

patterns-established:
  - "PEP-723 + open_master + threading.Event start-gate (verbatim copy from test_concurrent_reads_per_call_conn.py) — canonical Phase 65/65.1 concurrency-test template"
  - "Per-thread RNG seeded from thread_id for reproducible-but-distinct schedules across threads"
  - "Categorise function returns a tag; main bumps the per-category counter or appends to unknown_errors list with full repr(exc) for diagnostic dump"

requirements-completed: [REM-WR-04]

# Metrics
duration: 18min
completed: 2026-05-25
---

# Phase 65.1 Plan 05: WR-04 Concurrent Writes Coverage Summary

**Eight Python threads x ten mixed CREATE/DROP/ALTER ops on overlapping view names empirically pin the no-hang property and the five documented race classes (incl. TECH-DEBT 23 PK-violation + DuckDB tuple-deletion conflict) of the Phase 65 parser_override write path.**

## Performance

- **Duration:** ~18 min
- **Started:** 2026-05-25T18:00:00Z (approx)
- **Completed:** 2026-05-25T18:18:00Z (approx)
- **Tasks:** 1
- **Files modified:** 1 (created)

## Accomplishments

- New integration test `test/integration/test_concurrent_writes_per_call_conn.py` exercises 80 mixed-DDL ops across 8 threads in ~0.03s wall (well under the 30s budget)
- Empirically demonstrates documented races (constraint_violation + tuple_conflict) occur and are correctly absorbed without false-positive failures
- Documents a previously-uncatalogued PK-violation wording (commit-time TransactionContext shape) alongside the TECH-DEBT 23 single-statement check-time shape
- Documents the tuple-deletion conflict shape as the upstream serialisation contract for racing DROPs/ALTERs on the same _definitions row
- Final-state assertion catches spurious rows without over-pinning race ordering

## Task Commits

1. **Task 1: Create test_concurrent_writes_per_call_conn.py** — `4886212` (test)

_Verified stable across 5 back-to-back runs (each ~0.02–0.03s wall, all PASS with non-zero constraint_violation + tuple_conflict counts confirming the contention model is real)._

## Files Created/Modified

- `test/integration/test_concurrent_writes_per_call_conn.py` — 340 LOC; PEP-723 header (duckdb==1.5.2, pytest>=7.0); open_master + threading.Event gate verbatim from reads template; new WorkerResult dataclass with five buckets; categorise() helper distinguishing four documented races from unknown_error; ALTER uses IF EXISTS to silence DROP-then-ALTER no-ops

## Decisions Made

- **ALTER uses IF EXISTS**: a thread issuing ALTER on a view that another thread just DROPped becomes a silent no-op rather than surfacing "does not exist". Matches realistic bootstrap-retry script shapes and keeps the test's no-unknown-errors contract clean.
- **Five-bucket categorisation (plan asked for four)**: added `tuple_conflict` for DuckDB optimistic-concurrency rollback. This was discovered empirically during first execution and is documented upstream DuckDB behaviour — categorising it correctly is the contract-aligned move (see Deviations below).
- **Subset invariant over exact-equality**: race ordering forbids `final_views == union(creates) - union(drops)`. The robust invariant is `final_views ⊆ union(creates)` (no spurious rows).

## Deviations from Plan

### Auto-fixed Issues

**1. [Rule 1 - Bug] Plan's SQL did not match Snowflake/extension grammar**
- **Found during:** Task 1 first verification run
- **Issue:** Plan specified `CREATE SEMANTIC VIEW IF NOT EXISTS v TABLES (t1 AS t1) METRICS (t1.cnt AS count(*))` and `ALTER SEMANTIC VIEW v SET COMMENT 'x'`. Both fail Parser Error — CREATE requires `AS` keyword and a proper table-alias body (`TABLES (a AS t1 PRIMARY KEY (id)) DIMENSIONS (...) METRICS (...)`); ALTER requires `SET COMMENT = 'x'` (with `=`).
- **Fix:** Rewrote the CREATE statement to the canonical body shape (mirrors `test/integration/test_concurrent_ddl.py::CREATE_IF_NOT_EXISTS_SQL`); added `=` to the ALTER. Verified against `test/sql/65_alter_comment_merge_patch.test` and `test/sql/phase20_extended_ddl.test`.
- **Files modified:** test/integration/test_concurrent_writes_per_call_conn.py
- **Verification:** Test transitioned from 52 unknown_errors to 0 unknown_errors (with documented races correctly categorised)
- **Committed in:** 4886212

**2. [Rule 2 - Missing Critical] Categorise function missed the commit-time PK-violation wording**
- **Found during:** Task 1 second verification run
- **Issue:** Plan's categorise function only matched `Constraint Error ... Duplicate key ... primary key constraint` (the TECH-DEBT 23 single-statement check-time wording). The race-guard SQL rewrite uses a multi-statement form, so the PK violation surfaces at COMMIT time with a different wording: `TransactionContext Error: Failed to commit: PRIMARY KEY or UNIQUE constraint violation: duplicate key "v_X"`. Without this match, the test fails loudly on a documented race.
- **Fix:** Added second branch to categorise() matching `PRIMARY KEY or UNIQUE constraint violation` + `duplicate key`. Both shapes map to `constraint_violation`.
- **Files modified:** test/integration/test_concurrent_writes_per_call_conn.py
- **Verification:** All commit-time PK errors now correctly bucketed; zero unknown_errors
- **Committed in:** 4886212

**3. [Rule 2 - Missing Critical] Added fifth bucket for tuple_conflict**
- **Found during:** Task 1 second verification run
- **Issue:** Concurrent DROPs/ALTERs on the same view name occasionally raise `TransactionContext Error: Conflict on tuple deletion!` — DuckDB's optimistic-concurrency rollback when two transactions both wrote to the same row at commit time. Plan listed four buckets; this surfaces as a real fifth documented race shape.
- **Fix:** Added `tuple_conflict` field to WorkerResult, branch in categorise(), final summary print. Updated module docstring to document the bucket.
- **Files modified:** test/integration/test_concurrent_writes_per_call_conn.py
- **Verification:** All 8 conflict-on-tuple errors observed across runs now correctly bucketed; zero unknown_errors across 5 back-to-back runs
- **Committed in:** 4886212

**4. [Rule 3 - Blocking] Switch ALTER to IF EXISTS form**
- **Found during:** Task 1 first verification run
- **Issue:** Bare ALTER on a view a peer thread just DROPped raises "does not exist". Without IF EXISTS this would be a separate documented race shape (or require a sixth bucket); with IF EXISTS it's a silent no-op (the natural Snowflake-aligned bootstrap-retry pattern).
- **Fix:** Changed worker's ALTER branch to `ALTER SEMANTIC VIEW IF EXISTS {view} SET COMMENT = '...'`
- **Files modified:** test/integration/test_concurrent_writes_per_call_conn.py
- **Verification:** No "does not exist" errors observed across 5 runs; documented as decision in module docstring
- **Committed in:** 4886212

---

**Total deviations:** 4 auto-fixed (1 bug, 2 missing critical, 1 blocking)
**Impact on plan:** All four were essential to make the test actually pass against the live extension binary. The plan's per-call SQL was incorrect for the extension grammar (deviation 1), the categorise function under-specified the actual race shapes (deviations 2 + 3), and the ALTER form needed IF EXISTS to keep the test's contract clean (deviation 4). No scope creep — the test still implements exactly the design specified in the plan, just with corrected grammar and a more complete categorisation. The new bucket (tuple_conflict) and the commit-time PK wording are findings that the plan didn't anticipate; both are documented upstream DuckDB behaviour and should be folded back into TECH-DEBT 23's "known race shapes" wording (filed below).

## Issues Encountered

- **uv-related sandbox failure**: First `uv run` invocation in the default sandbox panicked inside the system-configuration crate (Tokio executor failed). Re-ran with `dangerouslyDisableSandbox: true` per CLAUDE.md Rule 2 — this is on the pre-approved bypass list for build/test commands. Subsequent runs all worked under the bypass.

## Threat Flags

None. The test exercises documented concurrency races — no new threat surface.

## Findings for Future Phases / TECH-DEBT

- **TECH-DEBT 23 wording should be extended** to mention BOTH PK-violation shapes:
  - Single-statement check-time (current wording): `Constraint Error: Duplicate key "name: v_X" violates primary key constraint`
  - Multi-statement commit-time (newly observed under race-guard rewrite): `TransactionContext Error: Failed to commit: PRIMARY KEY or UNIQUE constraint violation: duplicate key "v_X"`
- **A new TECH-DEBT entry candidate**: concurrent UPDATE/DELETE on the same `_definitions` row surfaces as `TransactionContext Error: Conflict on tuple deletion!`. This is DuckDB's documented optimistic-concurrency rollback (same shape any other DDL gets), but the user-facing wording could be improved. Out of scope for this plan; flagged for `_notes/v0.10.0_tech_debt_followups.md` if/when one is opened.

## Next Phase Readiness

- WR-04 finding closed via D-19 + D-20.
- Wave 1 status check: Plan 01 (test scaffolds) + Plan 02a (C ABI rework) + Plan 03a (BorrowedConnection ddl/ migration) + Plan 05 (this) all complete. Plan 02b (Wave 2) still owed — runtime ABI integration test verification deferred until then.
- Test file runs green against the May 25 extension binary (built post-Phase-65 close). When Plan 02b ships and a fresh build is produced, this test will be re-run as part of `just test-all` and should remain green (the test exercises only public DDL surface, not the WR-02 registration ABI).

---
*Phase: 65.1-phase-65-code-review-remediation*
*Completed: 2026-05-25*

## Self-Check: PASSED

- FOUND: test/integration/test_concurrent_writes_per_call_conn.py
- FOUND: .planning/phases/65.1-phase-65-code-review-remediation/65.1-05-SUMMARY.md
- FOUND: commit 4886212
