From ce5c6d093c795a1bf62d559235cbdd39e34adb84 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 06:59:05 -0400 Subject: [PATCH] review(Driver.Sql): stamp the fixing commit sha into the three Resolved findings --- code-reviews/Driver.Sql/findings.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code-reviews/Driver.Sql/findings.md b/code-reviews/Driver.Sql/findings.md index 63887fcb..a17eb89a 100644 --- a/code-reviews/Driver.Sql/findings.md +++ b/code-reviews/Driver.Sql/findings.md @@ -54,7 +54,7 @@ surface. **Recommendation:** Implement the documented fallback — when the dialect's answer is null/blank use the sole visible schema and only that one; with several visible schemas there is no defensible guess and the empty string must continue to resolve nothing. -**Resolution:** Fixed on this review branch (`review/sql`, 2026-08-10). `LoadAsync` now passes the already-loaded schema list into `ReadDefaultSchemaAsync`, which falls back to `schemas[0]` when exactly one schema is visible. TDD: `SqlCatalogDefaultSchemaTests` was written red (the unqualified tag read `BadNodeIdUnknown`) and carries a falsifiability control proving the ambiguous multi-schema case still rejects rather than guessing. +**Resolution:** Fixed on this review branch (`review/sql` `69ad376f`, 2026-08-10). `LoadAsync` now passes the already-loaded schema list into `ReadDefaultSchemaAsync`, which falls back to `schemas[0]` when exactly one schema is visible. TDD: `SqlCatalogDefaultSchemaTests` was written red (the unqualified tag read `BadNodeIdUnknown`) and carries a falsifiability control proving the ambiguous multi-schema case still rejects rather than guessing. --- @@ -88,7 +88,7 @@ surface. **Recommendation:** Correct both: nest the selector keys inside `rowSelector` in both examples with a warning that the top-level spelling is refused, and delete the `pollIntervalMs` row, stating where the interval actually comes from. -**Resolution:** Fixed on this review branch (`review/sql`, 2026-08-10). Both examples now use the nested `rowSelector` form and carry an explicit warning about the top-level spelling; the `pollIntervalMs` row is replaced by a paragraph naming the subscriber interval / `defaultPollInterval` / `PollGroupEngine.DefaultMinInterval` chain and recording that the field never existed. The scalar/string asymmetry between `whereValue` and `keyValue` (Driver.Sql.Contracts-001) is documented in the same edit. +**Resolution:** Fixed on this review branch (`review/sql` `69ad376f`, 2026-08-10). Both examples now use the nested `rowSelector` form and carry an explicit warning about the top-level spelling; the `pollIntervalMs` row is replaced by a paragraph naming the subscriber interval / `defaultPollInterval` / `PollGroupEngine.DefaultMinInterval` chain and recording that the field never existed. The scalar/string asymmetry between `whereValue` and `keyValue` (Driver.Sql.Contracts-001) is documented in the same edit. --- @@ -156,7 +156,7 @@ surface. **Recommendation:** Drop the reference (the `.Dialect` project reference already supplies it transitively for anything that needs it) and record why the project deliberately carries no provider package. -**Resolution:** Fixed on this review branch (`review/sql`, 2026-08-10). Reference removed and replaced with a comment stating the rule; the module's 365 tests still pass. +**Resolution:** Fixed on this review branch (`review/sql` `69ad376f`, 2026-08-10). Reference removed and replaced with a comment stating the rule; the module's 365 tests still pass. ---