Skip to content

Commit 28b0c8f

Browse files
blehnenclaude
andcommitted
docs(lessons): SQLite case-sensitive parameter-name binding (#155)
Capture the cross-transport gotcha surfaced by PR #156: System.Data.SQLite binds parameter names case-sensitively while Npgsql and Microsoft.Data.SqlClient do not, so a "@Status" vs "@status" mismatch only fails on SQLite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3b9f3cb commit 28b0c8f

1 file changed

Lines changed: 1 addition & 0 deletions

File tree

docs/lessons-learned.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,4 @@ Historical engineering notes from DotNetWorkQueue development. Moved here from `
5959
- **Phase scope reframing — RESEARCH should validate the phase isn't already done.** ROADMAP Phase 7 framed as "add XML doc comments to phases 2-4 public types." RESEARCH §1 surfaced that the builders had already added docs as they went. Phase 7 reframed to a VERIFICATION pass + csproj gate fixes (net8 `<DocumentationFile>` gap + ISSUE-032 NU1902 closure on Transport.SQLite). Pattern: when a phase title implies authoring, researcher should explicitly confirm the work was not already incidentally done before architect plans new authoring work.
6060
- **`<WarningsNotAsErrors>` pattern for accepted advisory carry-forward.** ISSUE-032 (OpenTelemetry NU1902 advisory) blocked the strictest `dotnet build -c Release -p:CI=true` build path on `Transport.SQLite`. Phase 7 PLAN-1.1 / commit `88ff8996` added `<WarningsNotAsErrors>NU1902</WarningsNotAsErrors>` to all 3 Release PropertyGroup blocks (`Release|net10.0`, `Release|net8.0`, `Release|AnyCPU`). The advisory still surfaces as a visible warning on every Release build (long-term remediation isn't forgotten) but the build is no longer blocked. Reusable pattern when a ship-blocking advisory is out of scope for the current milestone.
6161
- **SQLite + hold-transaction inbox pattern: structurally incompatible.** Inbox milestone (PR #143, issue #149) attempted to add `IRelationalWorkerNotification` to the SQLite transport alongside SqlServer + PostgreSQL. Failed on Jenkins across 14 CI runs in two recurring shapes: (1) `Sync_Commit`/`Async_Commit` NRE on `relational.Transaction` despite cast succeeding, (2) `Sync_Rollback`/`Async_Rollback`/`BusinessRow_*Visibility` handler-never-invoked 30s timeouts. Root cause is NOT a code bug — SQLite uses `BEGIN EXCLUSIVE`-on-write semantics. Holding the dequeue transaction for the duration of the user handler blocks ALL other writers on the queue table, including the worker thread's own next dequeue attempt → deadlock. `EnableHoldTransactionUntilMessageCommitted` has always been a no-op on SQLite for this reason. **Inbox pattern is a permanent non-goal for SQLite; outbox is viable with a documented concurrency caveat.** Treat the existing options-property as obsolete on SQLite (issue #149 tracks the cleanup). Don't re-attempt the four debugging directions tried in PR #143: lazy-options removal, property-injection, static `AsyncLocal<>`, factory-delegate wiring — none address the structural lock.
62+
- **System.Data.SQLite binds parameter names CASE-SENSITIVELY; Npgsql and Microsoft.Data.SqlClient do not.** Issue #155 (PR #156) — the shared `GetQueueCountQueryPrepareHandler` bound the status filter as `"@Status"` while all three transport SQL texts use lowercase `"@status"` (`... where status = @status`). PostgreSQL and SQL Server worked because their drivers match parameter names case-insensitively; System.Data.SQLite threw `SQLiteException: unknown error — Insufficient parameters supplied to the command` because it could not find a parameter named `@status`. The mismatch had existed indefinitely but was dormant: the status-filtered admin `Count` path was dead (no test ever enabled status on it), so it only surfaced once the #155 regression test exercised it. Fix (commit `3b9f3cb1`): align the `ParameterName` to the SQL placeholder's exact casing. Pattern: any handler that creates a parameter MUST match the SQL placeholder's case verbatim — assert the literal parameter name in unit tests (the `FakeCommandStringCache` mirrors the real lowercase SQL so the guard is meaningful), and never rely on driver-level case folding. Sibling lesson to the enum→int cast (issue #155, commit `c8093b65`): both bugs lived in the same dead code path, which is why repairing the test guard was as important as the fixes.

0 commit comments

Comments
 (0)