Add NuGet package READMEs and CodeFixes listing - #1
Merged
Conversation
- Add Quarry.Analyzers.CodeFixes to packages table in all READMEs - Create project-local READMEs for Quarry.Generator, Quarry.Analyzers, and Quarry.Analyzers.CodeFixes - Add standard Quarry header with logo to all sub-project READMEs - Add IsPackable=true to Generator, Analyzers, and Analyzers.CodeFixes csproj files - Fix Directory.Build.targets StripImgTags to use each project's own README via PackageReadmeSource property - Quarry project uses root README.md via PackageReadmeSource override Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Include logo-128.png in all packable projects via Directory.Build.targets and set PackageIcon in Directory.Build.props. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DJGosnell
added a commit
that referenced
this pull request
Mar 21, 2026
- Fix potential deadlock by reading stdout/stderr concurrently (#1) - Simplify output copy to always treat -o as a directory (#3) - Extract shared ResolveCsproj and FindMigrations into CommandHelpers (#4, #6) - Update MigrateCommands and BundleCommand to use shared helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DJGosnell
added a commit
that referenced
this pull request
Mar 21, 2026
- Fix potential deadlock by reading stdout/stderr concurrently (#1) - Simplify output copy to always treat -o as a directory (#3) - Extract shared ResolveCsproj and FindMigrations into CommandHelpers (#4, #6) - Update MigrateCommands and BundleCommand to use shared helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DJGosnell
added a commit
that referenced
this pull request
Mar 21, 2026
- Fix potential deadlock by reading stdout/stderr concurrently (#1) - Simplify output copy to always treat -o as a directory (#3) - Extract shared ResolveCsproj and FindMigrations into CommandHelpers (#4, #6) - Update MigrateCommands and BundleCommand to use shared helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DJGosnell
added a commit
that referenced
this pull request
Apr 6, 2026
… CTE diagnostics, EntityRef hardening Mid-pass-#2 remediation. Build is green; full test suite NOT yet run. Tasks 24 (new tests) and 29 (run + push) remain. - Consolidated ChainAnalyzer.ExtractShortTypeName + TransitionBodyEmitter.ExtractDtoShortName into a single CteNameHelpers.ExtractShortName helper that strips both global:: and namespace prefixes. Both call sites updated. Resolves the global-namespace DTO mismatch that would have re-introduced the captured-param drop bug. - Added QRY080 (CteInnerChainNotAnalyzable) and QRY081 (FromCteWithoutWith) descriptors; registered in s_deferredDescriptors; ChainAnalyzer reports via diagnostics?.Add() instead of PipelineErrorBag.Report (which had been surfacing user errors as QRY900 InternalError). - ProjectionAnalyzer.BuildColumnInfoFromTypeSymbol: tightened EntityRef foreign-key detection to (a) verify ContainingNamespace is Quarry and (b) unwrap Nullable<T> so EntityRef<X,Y>? still resolves as a foreign key. Added IsQuarryNamespace helper. - CallSiteBinder: when entity lookup misses AND raw.ContextClassName is null, fall back to AllContexts[0].Dialect when there is exactly one registered context (instead of always defaulting to PostgreSQL). - FileEmitter: IsCteInnerChain detection now scans all clause sites (was [0] only). - TransitionBodyEmitter: replaced fully-qualified System.Collections.Generic.Dictionary with `using` + short name. Documented multi-CTE limitation in EmitCteDefinition (matches first cteDef by name; #206 will resolve duplicate-DTO ambiguity). - ChainAnalyzer: documented the cteInnerResults span-key uniqueness invariant. Pending in next session: - Add diagnostic tests for QRY080/QRY081 (Test Quality #1) - Add Cte_FromCte_AllColumns test (Plan Compliance #1) - Add Cte_FromCte_CapturedParam reuse-prepared assertion (Test Quality #2) - Add global-namespace DTO regression test (Correctness #1 verification) - Run full suite, commit (non-WIP), push, verify CI
DJGosnell
added a commit
that referenced
this pull request
Apr 6, 2026
…ed diagnostics Closes the pass #2 remediation work that began in WIP commit 61a0ee5. The source-side fixes from that commit are unchanged; this commit adds the diagnostic and coverage tests that pass #2 identified as gaps and folds in a classification correction. Source remediation (carried over from 61a0ee5; CI verified at 243034d): - CteNameHelpers.ExtractShortName consolidated from two divergent local helpers in ChainAnalyzer and TransitionBodyEmitter. Strips both `global::` and namespace prefixes so global-namespace DTOs no longer trigger a silent captured-param drop (Correctness #1). - QRY080 CteInnerChainNotAnalyzable + QRY081 FromCteWithoutWith dedicated diagnostic descriptors. ChainAnalyzer reports via `diagnostics?.Add(...)` instead of routing user-input errors through QRY900 InternalError (Codebase Consistency #4). - ProjectionAnalyzer.BuildColumnInfoFromTypeSymbol EntityRef FK detection tightened to verify `ContainingNamespace == Quarry` and unwrap `Nullable<T>` so `EntityRef<X,Y>?` still resolves as a foreign key (Correctness #2/#3). - CallSiteBinder single-context dialect fallback when ContextClassName is null AND AllContexts.Length == 1 (Correctness #4). - IsCteInnerChain detection in FileEmitter now scans all clause sites instead of inspecting only ClauseSites[0] (Codebase Consistency #5). - Documented cteInnerResults span-key uniqueness invariant and the multi-CTE first-match limitation in EmitCteDefinition (Correctness #5/#6, #206 tracks the multi-CTE work). - TransitionBodyEmitter switched to short-name `Dictionary<,>?` parameter via `using System.Collections.Generic` (Codebase Consistency #2). Tests added (this commit): - Cte_With_NonInlineInnerArgument_EmitsQRY080: passes a field-reference inner argument to With<T>() so DetectCteInnerChain cannot classify it; asserts QRY080 fires and QRY900 does not (Test Quality #1). - Cte_FromCte_WithoutPrecedingWith_EmitsQRY081: FromCte<T>() with no preceding With<T>(); asserts QRY081 fires and QRY900 does not (Test Quality #1). - Cte_With_GlobalNamespaceDto_StripsGlobalPrefix: two-source compilation with a `global::GlobalOrderDto`; asserts the generated SQL constant uses the bare `GlobalOrderDto` name (verbatim-quoted form) and that the captured-param copy `Pn = __inner.P0` is emitted. Regression for the Correctness #1 silent failure mode. - Cte_FromCte_AllColumns: identity FromCte<Order>().Select(o => o) across all 4 dialects, vs the existing tuple-projection tests (Plan Compliance #1). Test reclassification: - Cte_FromCte_CapturedParam was marked Test Quality #2 (A) for "re-execute the SAME prepared chain after mutation". On investigation, PreparedQuery in this codebase is a SNAPSHOT at chain construction — the generated `Where_xxx` interceptor extracts the captured variable into the carrier P0 field at the `.Where()` call, before `Prepare()` runs, and there is no Bind/SetParameter API to re-execute the same instance with a new value. The lt2 pattern is the correct way to test "different captured value" semantics. The test is unchanged in behavior but the comment now documents the snapshot semantics. Reclassified A→D in review.md. Test totals: 2782 main + 103 analyzer + 79 migration = 2964 passing. Manifests regenerated to include the new Cte_FromCte_AllColumns chain.
DJGosnell
added a commit
that referenced
this pull request
Apr 6, 2026
… CTE diagnostics, EntityRef hardening Mid-pass-#2 remediation. Build is green; full test suite NOT yet run. Tasks 24 (new tests) and 29 (run + push) remain. - Consolidated ChainAnalyzer.ExtractShortTypeName + TransitionBodyEmitter.ExtractDtoShortName into a single CteNameHelpers.ExtractShortName helper that strips both global:: and namespace prefixes. Both call sites updated. Resolves the global-namespace DTO mismatch that would have re-introduced the captured-param drop bug. - Added QRY080 (CteInnerChainNotAnalyzable) and QRY081 (FromCteWithoutWith) descriptors; registered in s_deferredDescriptors; ChainAnalyzer reports via diagnostics?.Add() instead of PipelineErrorBag.Report (which had been surfacing user errors as QRY900 InternalError). - ProjectionAnalyzer.BuildColumnInfoFromTypeSymbol: tightened EntityRef foreign-key detection to (a) verify ContainingNamespace is Quarry and (b) unwrap Nullable<T> so EntityRef<X,Y>? still resolves as a foreign key. Added IsQuarryNamespace helper. - CallSiteBinder: when entity lookup misses AND raw.ContextClassName is null, fall back to AllContexts[0].Dialect when there is exactly one registered context (instead of always defaulting to PostgreSQL). - FileEmitter: IsCteInnerChain detection now scans all clause sites (was [0] only). - TransitionBodyEmitter: replaced fully-qualified System.Collections.Generic.Dictionary with `using` + short name. Documented multi-CTE limitation in EmitCteDefinition (matches first cteDef by name; #206 will resolve duplicate-DTO ambiguity). - ChainAnalyzer: documented the cteInnerResults span-key uniqueness invariant. Pending in next session: - Add diagnostic tests for QRY080/QRY081 (Test Quality #1) - Add Cte_FromCte_AllColumns test (Plan Compliance #1) - Add Cte_FromCte_CapturedParam reuse-prepared assertion (Test Quality #2) - Add global-namespace DTO regression test (Correctness #1 verification) - Run full suite, commit (non-WIP), push, verify CI
DJGosnell
added a commit
that referenced
this pull request
Apr 6, 2026
…ed diagnostics Closes the pass #2 remediation work that began in WIP commit 61a0ee5. The source-side fixes from that commit are unchanged; this commit adds the diagnostic and coverage tests that pass #2 identified as gaps and folds in a classification correction. Source remediation (carried over from 61a0ee5; CI verified at 243034d): - CteNameHelpers.ExtractShortName consolidated from two divergent local helpers in ChainAnalyzer and TransitionBodyEmitter. Strips both `global::` and namespace prefixes so global-namespace DTOs no longer trigger a silent captured-param drop (Correctness #1). - QRY080 CteInnerChainNotAnalyzable + QRY081 FromCteWithoutWith dedicated diagnostic descriptors. ChainAnalyzer reports via `diagnostics?.Add(...)` instead of routing user-input errors through QRY900 InternalError (Codebase Consistency #4). - ProjectionAnalyzer.BuildColumnInfoFromTypeSymbol EntityRef FK detection tightened to verify `ContainingNamespace == Quarry` and unwrap `Nullable<T>` so `EntityRef<X,Y>?` still resolves as a foreign key (Correctness #2/#3). - CallSiteBinder single-context dialect fallback when ContextClassName is null AND AllContexts.Length == 1 (Correctness #4). - IsCteInnerChain detection in FileEmitter now scans all clause sites instead of inspecting only ClauseSites[0] (Codebase Consistency #5). - Documented cteInnerResults span-key uniqueness invariant and the multi-CTE first-match limitation in EmitCteDefinition (Correctness #5/#6, #206 tracks the multi-CTE work). - TransitionBodyEmitter switched to short-name `Dictionary<,>?` parameter via `using System.Collections.Generic` (Codebase Consistency #2). Tests added (this commit): - Cte_With_NonInlineInnerArgument_EmitsQRY080: passes a field-reference inner argument to With<T>() so DetectCteInnerChain cannot classify it; asserts QRY080 fires and QRY900 does not (Test Quality #1). - Cte_FromCte_WithoutPrecedingWith_EmitsQRY081: FromCte<T>() with no preceding With<T>(); asserts QRY081 fires and QRY900 does not (Test Quality #1). - Cte_With_GlobalNamespaceDto_StripsGlobalPrefix: two-source compilation with a `global::GlobalOrderDto`; asserts the generated SQL constant uses the bare `GlobalOrderDto` name (verbatim-quoted form) and that the captured-param copy `Pn = __inner.P0` is emitted. Regression for the Correctness #1 silent failure mode. - Cte_FromCte_AllColumns: identity FromCte<Order>().Select(o => o) across all 4 dialects, vs the existing tuple-projection tests (Plan Compliance #1). Test reclassification: - Cte_FromCte_CapturedParam was marked Test Quality #2 (A) for "re-execute the SAME prepared chain after mutation". On investigation, PreparedQuery in this codebase is a SNAPSHOT at chain construction — the generated `Where_xxx` interceptor extracts the captured variable into the carrier P0 field at the `.Where()` call, before `Prepare()` runs, and there is no Bind/SetParameter API to re-execute the same instance with a new value. The lt2 pattern is the correct way to test "different captured value" semantics. The test is unchanged in behavior but the comment now documents the snapshot semantics. Reclassified A→D in review.md. Test totals: 2782 main + 103 analyzer + 79 migration = 2964 passing. Manifests regenerated to include the new Cte_FromCte_AllColumns chain.
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
Addresses review.md classifications from session 2: - A/#4 Correctness: extend HasQuarryContextAttributeSyntactic with an AliasQualifiedNameSyntax branch so [global::QuarryContextAttribute] and extern-alias attribute forms pass the syntactic pre-filter. - B/#1 Plan: drop RawSqlTypeInfo.FullyQualifiedResultTypeName — written but never read; ResultTypeName already carries the final display form (FQN for nested, short for not). - B/#7 Tests: add alias-qualified attribute QRY044 coverage. - B/#8 Tests: add struct-with-init-only-property QRY043 coverage. - B/#9 Tests: add mixed RawSqlScalarAsync<int> + failing RawSqlAsync<T> test exercising the scalar branch in PipelineOrchestrator + FileEmitter. 3259 tests pass (+3 new).
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
Previously, Sql.Raw<T> used inside a Select tuple/DTO/object-init projection
silently rendered as an empty string literal in the generated SQL
(e.g., SELECT "OrderId", "" FROM "orders") because ProjectionAnalyzer fell
through to the generic fallback where columnName="" and the raw C# source
was placed in SqlExpression then later stripped.
Add a Raw case to GetAggregateInfo that parses each template argument via
SqlExprParser, walks the resulting SqlExpr tree with a projection-aware
renderer, and substitutes {0}/{1}/... template placeholders with canonical
{ColumnName} identifier placeholders (dialect-resolved later), @__proj{N}
parameter placeholders for captured runtime vars, and inline literals for
compile-time constants. Binary ops, unary ops, function calls, IS NULL, IN,
and LIKE expressions render inline so args like u.Price * 2 are supported.
The generic T type argument on Sql.Raw<T> determines the column CLR type;
template/arg count mismatches fail via RawCallExpr.Validate().
Covers tuple, DTO, object-initializer, and single-column projection forms
for single-entity Select (joined projections follow in a separate commit).
Task #1 of 3 in #256 workflow.
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
Review session 2 finding #1. Where-path `Sql.Raw` calls surface template errors (placeholder/ argument count mismatch) as QRY029 compile-time errors via `PipelineOrchestrator.CollectTranslatedDiagnostics`, because the raw site's `Expression` is a `RawCallExpr` that runs `Validate()`. Select- projection `Sql.Raw` calls were silent — `ProjectionAnalyzer. BuildSqlRawInfo` validated via a transient `RawCallExpr` shell and, on failure, discarded both the shell and the error, returning `(null, null)`. The projection then failed analysis and the chain degraded to runtime build, leaving no user-visible diagnostic. `GetRawTemplateValidationError` now returns the `Validate()` message (the same string the Where path shows). `BuildSqlRawInfo` records the message via a thread-static accumulator modeled on `PipelineErrorBag`. Public entry points in `ProjectionAnalyzer` drain the accumulator and attach the messages to the returned `ProjectionInfo` via a new init- only `SqlRawValidationErrors` property. `PipelineOrchestrator` emits QRY029 per entry, scoped to the Select call's location. Scope note: QRY029 is attached to the Select call location rather than the individual `Sql.Raw` call site — less precise than the Where path, but sufficient to locate the bad template in the lambda. Added `ProjectionFailureReason.SqlRawValidationError` for future callers that want to distinguish this degradation class; current pipeline reads from `SqlRawValidationErrors` directly.
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
Addresses review.md classifications from session 2: - A/#4 Correctness: extend HasQuarryContextAttributeSyntactic with an AliasQualifiedNameSyntax branch so [global::QuarryContextAttribute] and extern-alias attribute forms pass the syntactic pre-filter. - B/#1 Plan: drop RawSqlTypeInfo.FullyQualifiedResultTypeName — written but never read; ResultTypeName already carries the final display form (FQN for nested, short for not). - B/#7 Tests: add alias-qualified attribute QRY044 coverage. - B/#8 Tests: add struct-with-init-only-property QRY043 coverage. - B/#9 Tests: add mixed RawSqlScalarAsync<int> + failing RawSqlAsync<T> test exercising the scalar branch in PipelineOrchestrator + FileEmitter. 3259 tests pass (+3 new).
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
* Support Sql.Raw<T> in single-entity Select projections (#256) Previously, Sql.Raw<T> used inside a Select tuple/DTO/object-init projection silently rendered as an empty string literal in the generated SQL (e.g., SELECT "OrderId", "" FROM "orders") because ProjectionAnalyzer fell through to the generic fallback where columnName="" and the raw C# source was placed in SqlExpression then later stripped. Add a Raw case to GetAggregateInfo that parses each template argument via SqlExprParser, walks the resulting SqlExpr tree with a projection-aware renderer, and substitutes {0}/{1}/... template placeholders with canonical {ColumnName} identifier placeholders (dialect-resolved later), @__proj{N} parameter placeholders for captured runtime vars, and inline literals for compile-time constants. Binary ops, unary ops, function calls, IS NULL, IN, and LIKE expressions render inline so args like u.Price * 2 are supported. The generic T type argument on Sql.Raw<T> determines the column CLR type; template/arg count mismatches fail via RawCallExpr.Validate(). Covers tuple, DTO, object-initializer, and single-column projection forms for single-entity Select (joined projections follow in a separate commit). Task #1 of 3 in #256 workflow. * Support Sql.Raw<T> in joined and single-column Select projections (#256) Add a Raw case to GetJoinedAggregateInfo that resolves column references to canonical {alias}.{ColumnName} placeholders using the per-parameter lookup. The shared BuildSqlRawInfo helper and the SqlExpr tree walker from Phase 1 are reused via a column-resolver delegate — only the column resolution differs between single-entity and joined contexts. Single-column projections (e.g., .Select(u => Sql.Raw<string>(...))) already route through GetAggregateInfo via AnalyzeInvocation, so no additional change is needed; joined single-column does the same via AnalyzeJoinedInvocation → ResolveJoinedAggregate → GetJoinedAggregateInfo. Add sanity tests for joined-tuple and single-column forms; the existing column-reference sanity test from Phase 1 covers single-entity tuple form. Task #2 of 3 in #256 workflow. * Cross-dialect test coverage for Sql.Raw<T> in Select projections (#256) Add 6 tests to complement the sanity tests from Phases 1+2, covering the full surface of the fix: - Multiple column references — Sql.Raw<string>("coalesce({0}, {1})", ...) - Captured variable — verifies @__proj{N} local placeholders are remapped to dialect-specific parameter placeholders (@p0 / $1 / ?) - Literal parameter — compile-time constant inlines as SQL literal - No placeholders — template with no {N} substitutions passes through - DTO initializer — new UserSummaryDto { ... = Sql.Raw<string>(...) } - Binary op arg — u.UserId * 10 exercises the IR-based arg tree walker All 9 tests (3 from Phases 1+2 + 6 added here) assert SQL output across SQLite, PostgreSQL, MySQL, and SqlServer. Task #3 of 3 in #256 workflow. * Remediate review findings for Sql.Raw<T> Select projection (#256) Address review findings from _sessions/256-fix-sql-raw-select-projection/review.md: - Fail-loud fallback (#3): AnalyzeProjectedExpression returns null for a methodName=="Raw" whose GetAggregateInfo returned null, preventing the generic type-info fallback from producing an empty-column ProjectedColumn that would regress the original #256 bug. AnalyzeInvocation emits a specific CreateFailed message for the single-column path. - Captured-var typing (#4, #8, #16): RenderRawArgToCanonical tries ResolveScalarArgSql first for simple scalar args (identifiers, literals, captured members). This consults the semantic model for authoritative CLR types rather than the SqlExprParser default of "object", so a captured DateTime landed in ParameterInfo as "object" becomes "DateTime". - Dialect-aware booleans (#5): FormatLiteralForProjection emits canonical {@boolt}/{@boolf} placeholders rather than per-dialect bool text. QuoteSqlExpression resolves the placeholder to TRUE/FALSE on PostgreSQL and 1/0 elsewhere. Because projection analysis runs at a fixed discovery dialect, emitting dialect-rendered text at analysis time would embed the wrong value in the cached SqlExpression. - String-concat guard (#6): RenderRawArgNode's Add walker bails to null if either operand is a string/char typed literal or captured; the projection fails loudly rather than emitting "a + b" which is invalid on MySQL/SqlServer. Users who want string concat should write it in the template text. - Validation refactor (#7): Replaced the throwaway RawCallExpr shell with a dedicated IsRawTemplateValid helper. Transient shell is now isolated to one helper with clear scope. - T="object" guard (#21): TryExtractSqlRawTypeArg returns null rather than "object" when T is unresolvable. BuildSqlRawInfo then fails the projection loudly instead of letting ChainAnalyzer.TryResolveAggregateTypeFromSql misinfer a CLR type from SUM/MIN/MAX substrings in the user's template. - Tests: Add Select_SqlRaw_BooleanLiteralArg_DialectAware (#5, #13) and Select_SqlRaw_CapturedVariable_TypeInferredFromSemanticModel (#4, #12, #14). Review classifications + Action Taken recorded in _sessions/256-fix-sql-raw-select-projection/review.md §Classifications. All 3253 tests pass (3242 baseline + 11 Sql.Raw projection tests). * Record PR #262 in workflow state (#256) * Back-step REMEDIATE → REVIEW for session 2 re-analysis (#256) User declined to finalize PR #262 and requested a fresh full review. Archived prior review.md to review-session1.md; workflow phase reset to REVIEW. * Make Sql.Raw projection walker fail loudly on unsupported args (#256) Review session 2 findings #4, #8. The projection-arg walker used to emit `SqlRawExpr.SqlText` verbatim when it reached a node the parser could not translate. Since `SqlExprParser` falls through to `new SqlRawExpr(expression.ToString())` for C# ternaries, unknown invocations, postfix unary (other than `!`), array-creation without initializer, element access, and interpolated strings, any of those inside `Sql.Raw<T>(template, arg)` would leak the C# source text — e.g. `/* unsupported: C# ternary expression */` or `u.Foo(x)` — into the generated SQL. This reproduces the exact class of silent-wrong-SQL bug this PR fixes. The walker now returns null for `SqlRawExpr`, forcing the caller to bail out. The string-concat Add guard now also checks `ColumnRefExpr` operands via a new `isStringColumn` delegate threaded alongside the existing column resolver. Previously only direct string literals and captured string locals were caught, so `Sql.Raw<string>("{0}", u.FirstName + u.LastName)` would still emit `("FirstName" + "LastName")` — invalid on MySQL and SqlServer. The guard now bails on any string-typed operand: literal, captured, or column. Both `RenderRawArgToCanonical` and `RenderRawArgToCanonicalJoined` receive an `isStringColumn` delegate that closes over the column lookup for the current context; `IsStringColumnRef` and `IsJoinedStringColumnRef` implement the two contexts. * Propagate IsStaticField and ExpressionPath for Sql.Raw captured vars (#256) Review session 2 findings #2, #9, #17. `AddCapturedAsProjectionParameter` now mirrors the ParameterInfo construction in `SqlExprClauseTranslator.ExtractParametersCore` (SqlExprClauseTranslator.cs:88-92) so captured-variable metadata propagates consistently across the Where-path and Select-projection paths: - `ExpressionPath = captured.ExpressionPath` — needed by the parameter binder to emit direct-path navigation code for deep captures like `obj.Inner.Field`. - `IsStaticCapture = captured.IsStaticField` — controls whether UnsafeAccessor uses StaticField kind (null target) vs Field kind (func.Target). Latent today: the walker path is only reached for captured variables that are NOT direct identifiers or parameter-prefixed member accesses (the fast-path in RenderRawArgToCanonical delegates those to ResolveScalarArgSql). Remaining walker captures are rare (function-call args, operator arms) and SqlExprParser never sets IsStaticField=true in the current code. The fix aligns the projection path with the canonical pattern so future parser changes or more complex Raw arg shapes don't silently drop capture metadata. * Tighten bool-literal detection and operator-table fallback (#256) Review session 2 findings #6, #7. `FormatLiteralForProjection` previously treated `"TRUE"`, `"true"`, and `"1"` as truthy, but `SqlExprParser.ParseLiteral` only ever emits `"TRUE"` or `"FALSE"` for bool literals (SqlExprParser.cs:270-274). The extra checks were dead and added cognitive load — now only `"TRUE"` maps to `{@boolt}`, everything else to `{@boolf}`. `GetRawBinaryOperator` used to return `"?"` for any unmapped `SqlBinaryOperator`, which is the MySQL parameter placeholder and would parse into unrelated runtime behavior — the exact silent- wrong-SQL failure mode this PR fixes. It now throws `ArgumentOutOfRangeException`, so any future operator addition that forgets to update the table will fail the build rather than emit garbage SQL. * Emit QRY029 for Sql.Raw template errors in Select projections (#256) Review session 2 finding #1. Where-path `Sql.Raw` calls surface template errors (placeholder/ argument count mismatch) as QRY029 compile-time errors via `PipelineOrchestrator.CollectTranslatedDiagnostics`, because the raw site's `Expression` is a `RawCallExpr` that runs `Validate()`. Select- projection `Sql.Raw` calls were silent — `ProjectionAnalyzer. BuildSqlRawInfo` validated via a transient `RawCallExpr` shell and, on failure, discarded both the shell and the error, returning `(null, null)`. The projection then failed analysis and the chain degraded to runtime build, leaving no user-visible diagnostic. `GetRawTemplateValidationError` now returns the `Validate()` message (the same string the Where path shows). `BuildSqlRawInfo` records the message via a thread-static accumulator modeled on `PipelineErrorBag`. Public entry points in `ProjectionAnalyzer` drain the accumulator and attach the messages to the returned `ProjectionInfo` via a new init- only `SqlRawValidationErrors` property. `PipelineOrchestrator` emits QRY029 per entry, scoped to the Select call's location. Scope note: QRY029 is attached to the Select call location rather than the individual `Sql.Raw` call site — less precise than the Where path, but sufficient to locate the bad template in the lambda. Added `ProjectionFailureReason.SqlRawValidationError` for future callers that want to distinguish this degradation class; current pipeline reads from `SqlRawValidationErrors` directly. * Test remediation coverage for Sql.Raw Select projection (#256) Review session 2 findings #11, #12, #14. UsageSiteDiscoveryTests.cs (#11): Three new QRY029 tests for projection-path Sql.Raw: - Too many arguments - Too few arguments - Non-sequential placeholders ({0}, {2} skipping {1}) All assert QRY029 fires and the message matches the RawCallExpr.Validate text, mirroring the existing Where-path tests. CrossDialectMiscTests.cs (#12, #14): - Select_SqlRaw_CapturedVariable_TypeInferredFromSemanticModel now also asserts Name, Value, IsCollection, IsEnum, and executes the query at runtime to verify the captured DateTime round-trips through the parameter binder (shallow-test remediation). - Select_SqlRaw_Joined_MultipleArgs — joined projection with three args from a literal, u.UserName, and o.Status. Exercises ResolveJoinedColumnRefToPlaceholder for two distinct lambda params. - Select_SqlRaw_Joined_WithCapturedVariable — joined projection with a captured int threshold in the Raw arg. Exercises IsScalarArgCandidateJoined and the fast-path delegation in the joined context. Manifest regeneration reflects the three new cross-dialect queries. * Share binary-operator table between SqlExprRenderer and Sql.Raw walker (#256) Review session 2 finding #16 (scoped). The Sql.Raw projection walker used to carry its own copy of the SqlBinaryOperator → text table (`GetRawBinaryOperator`). Any future operator addition would require updating both it and `SqlExprRenderer.GetSqlOperator`, and any dialect-specific fix in the renderer (e.g., the string-concat TODO at SqlExprRenderer.cs:290) would bypass the walker. `GetSqlOperator` is now `internal` and called directly from the walker. The renderer's fallback was also tightened to throw `ArgumentOutOfRangeException` rather than emit `"?"` — matching the walker's stricter contract and preventing an unmapped operator from silently rendering as the MySQL parameter placeholder. Full consolidation of the remaining shape-level duplication (IN/LIKE/IS NULL/function-call/unary emission) is out of scope: those render paths in SqlExprRenderer invoke dialect-sensitive recursion that the canonical walker must not enter. Tracked as a follow-up in the walker's docstring: to share the rest, SqlExprRenderer would need a canonical-projection output mode with delegate hooks for column resolution, captured-value accumulation, and literal rendering. * Record Action Taken for session 2 review findings (#256) Populates the review.md Classifications table with a concise description of how each A/B finding was addressed (or why D findings were left alone). Session 2 remediation complete — ready to finalize. * Save PR #262 body draft alongside session artifacts (#256) * Update workflow.md for session 2 REMEDIATE completion (#256) * chore: remove session artifacts before merge
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
Addresses review.md classifications from session 2: - A/#4 Correctness: extend HasQuarryContextAttributeSyntactic with an AliasQualifiedNameSyntax branch so [global::QuarryContextAttribute] and extern-alias attribute forms pass the syntactic pre-filter. - B/#1 Plan: drop RawSqlTypeInfo.FullyQualifiedResultTypeName — written but never read; ResultTypeName already carries the final display form (FQN for nested, short for not). - B/#7 Tests: add alias-qualified attribute QRY044 coverage. - B/#8 Tests: add struct-with-init-only-property QRY043 coverage. - B/#9 Tests: add mixed RawSqlScalarAsync<int> + failing RawSqlAsync<T> test exercising the scalar branch in PipelineOrchestrator + FileEmitter. 3259 tests pass (+3 new).
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
Addresses review.md classifications from session 2: - A/#4 Correctness: extend HasQuarryContextAttributeSyntactic with an AliasQualifiedNameSyntax branch so [global::QuarryContextAttribute] and extern-alias attribute forms pass the syntactic pre-filter. - B/#1 Plan: drop RawSqlTypeInfo.FullyQualifiedResultTypeName — written but never read; ResultTypeName already carries the final display form (FQN for nested, short for not). - B/#7 Tests: add alias-qualified attribute QRY044 coverage. - B/#8 Tests: add struct-with-init-only-property QRY043 coverage. - B/#9 Tests: add mixed RawSqlScalarAsync<int> + failing RawSqlAsync<T> test exercising the scalar branch in PipelineOrchestrator + FileEmitter. 3259 tests pass (+3 new).
DJGosnell
added a commit
that referenced
this pull request
Apr 23, 2026
* feat(generator): QRY043 diagnostic for un-materializable RawSqlAsync row types
Surface the real reason RawSqlAsync<T> row types fail to compile when T is a
positional record or has init-only properties. Previously authors saw cryptic
CS7036/CS8852 errors against generated code; now they see QRY043 naming their
type and the specific shape violation.
Detection runs in DisplayClassEnricher where the ITypeSymbol is already resolved
via the supplemental compilation. Emission is suppressed for affected sites so
QRY043 is the only error reported.
* feat(generator): support nested row-entity types in RawSqlAsync interceptors
Nested row types (row record declared inside an enclosing class) previously broke
the generator: it emitted `using <EnclosingType>;` which the compiler rejects
with CS0138. Fix tracks the nesting state on RawSqlTypeInfo and emits the
`global::`-prefixed fully qualified type name in generated bodies for nested
types so references resolve without a using directive. Namespace-level row
types still use their short name + using (unchanged codegen).
* feat(packaging): ship Quarry.targets auto-registering Quarry.Generated
Adds build/Quarry.targets to the Quarry NuGet package. It appends
Quarry.Generated to <InterceptorsNamespaces> so consumers no longer hit CS9137
for the Quarry-internal namespace they can't reasonably discover. Consumers
still add their own QuarryContext namespace — Phase 4's QRY044 analyzer
surfaces that gap at authoring time.
Also exposes InterceptorsNamespaces as a CompilerVisibleProperty so that
analyzer can read it from AnalyzerConfigOptions.
* feat(analyzers): QRY044 warns when QuarryContext namespace is missing from InterceptorsNamespaces
Surfaces the CS9137 project-setup gap at authoring time with the exact
<InterceptorsNamespaces> line to paste into the .csproj. Warning severity:
the build would fail with CS9137 anyway, so this is an earlier signal, not a
new error. Context classes in the global namespace are ignored because
Quarry.Generated (auto-registered by the shipped targets file) already
covers that path.
Descriptive diagnostic only — no CodeFixProvider, since the fix target is
the .csproj rather than a source document and standard Roslyn code fixes
can't reliably modify project files.
* docs: document row-entity shape requirements and interceptor opt-in behavior
Updates llm.md and Quarry.Generator/{README.md,llm.md} with:
- Revised InterceptorsNamespaces guidance: Quarry.Generated is now
auto-registered by the shipped targets file, so the doc only prompts for
the consumer's context namespace.
- New Row entity shape note under Raw SQL describing the parameterless ctor
+ public get/set property requirements, with QRY043 and the chain-query
workaround for immutable shapes.
- Nested row types are explicitly called out as supported.
- QRY043 and QRY044 added to the diagnostic inventory tables.
* fix(generator): remediate review findings for #259
Review items addressed inline:
- #3 (B, Correctness): `CheckRowEntityMaterializability` now also rejects
abstract classes and interfaces. CS0144 would otherwise fire against the
generated `new T()` with no indication of which row type was at fault.
- #5 (B, Tests): adds a nested-row test driving the struct-reader fallback
by using an expression SELECT list, exercising `SanitizeForIdentifier`
and the FQN-in-`IRowReader<T>` path.
- #6 (B, Tests): strengthens the namespace-level-row regression to assert
that `using TestApp.Rows;` IS emitted.
- #7 (B, Tests): adds a QRY044 test where `build_property.InterceptorsNamespaces`
is entirely absent (null), confirming the diagnostic still fires.
- Plus QRY043 tests for abstract and interface row types.
Test totals: 3256 passing (+4 from Phase 5 green baseline).
* chore: record PR #260 in workflow.md + add pr-body.md session artifact
* docs: include abstract class and interface rejection in QRY043 docs
The REMEDIATE-phase extension of CheckRowEntityMaterializability added two
additional rejection cases (abstract classes and interfaces) that weren't
reflected in llm.md, src/Quarry.Generator/README.md, or
src/Quarry.Generator/llm.md. Brings the guide text and diagnostic inventory
tables in line with the actual QRY043 behavior.
* chore(session): record session 2 resume and populate review Action Taken
Resume bookkeeping only — no code changes. Closes the review classification
loop by filling Action Taken for B items #3/#5/#6/#7 with references to
commit be224dd; adds session log entry covering the resume.
* chore(session): back-step REMEDIATE -> REVIEW for re-analysis
* fix: remediate second-round review findings (A/#4, B/#1/#7/#8/#9)
Addresses review.md classifications from session 2:
- A/#4 Correctness: extend HasQuarryContextAttributeSyntactic with an
AliasQualifiedNameSyntax branch so [global::QuarryContextAttribute]
and extern-alias attribute forms pass the syntactic pre-filter.
- B/#1 Plan: drop RawSqlTypeInfo.FullyQualifiedResultTypeName — written
but never read; ResultTypeName already carries the final display form
(FQN for nested, short for not).
- B/#7 Tests: add alias-qualified attribute QRY044 coverage.
- B/#8 Tests: add struct-with-init-only-property QRY043 coverage.
- B/#9 Tests: add mixed RawSqlScalarAsync<int> + failing RawSqlAsync<T>
test exercising the scalar branch in PipelineOrchestrator + FileEmitter.
3259 tests pass (+3 new).
* docs(pr): update PR body for second remediation round (A/#4, B/#1/#7/#8/#9)
* chore(session): log rebase on origin/master (4daf62d)
* chore(session): log second rebase on origin/master (e4354a4)
* chore(session): log third rebase on origin/master (95c4bbb + d2a6b1e)
* chore: remove session artifacts before merge
DJGosnell
added a commit
that referenced
this pull request
Apr 25, 2026
…al SqlConnection on Ss + cross-dialect mirror (#270) (#276) * Add Testcontainers.MsSql + MsSqlTestContainer skeleton + Docker probe (#270) Phase 1 of mirroring PR #266's PG execution coverage to SQL Server. Adds Testcontainers.MsSql 4.* to Quarry.Tests, a lazy process-wide MsSqlContainer helper that mirrors PostgresTestContainer's shape, and a single regression-probe test confirming the MS SQL 2022 container boots and accepts a connection. EnsureBaselineAsync / CreateOwnedSchemaAsync are stubbed with NotImplementedException — Phase 2 will implement the schema DDL, quarry_test_user mapped login, and seed data, then upgrade QueryTestHarness.Ss off MockDbConnection. Tests: - MsSqlContainerSmokeTests.SqlServerContainer_BootsAndAcceptsConnection: boots the container, runs SELECT @@Version, asserts the banner contains "SQL Server". The smoke test pays the ~30s cold-start cost only once per process; the shared container is reused across all later SQL Server integration tests by virtue of MsSqlTestContainer's static-field caching. * Port schema DDL to SQL Server + upgrade QueryTestHarness.Ss to real SqlConnection (#270) Phase 2 of the SQL Server execution-mirror work. QueryTestHarness.Ss now attaches to a real Microsoft.Data.SqlClient SqlConnection against the shared Testcontainers SQL Server 2022 container instead of MockDbConnection. Lite, Pg, and Ss are all on real providers; My stays on the mock (covered by the parallel issue #269). Isolation strategy is transactional by default and mirrors PG's pattern: - MsSqlTestContainer.EnsureBaselineAsync() creates the shared quarry_test schema, the quarry_test_user login + db user (default schema = quarry_test, db_owner role), the test tables, and the seed data — all gated by sp_getapplock so concurrent test processes can share one container without racing on setup. - Each CreateAsync() opens a fresh pooled SqlConnection authenticated as quarry_test_user. The default-schema mapping makes unqualified [users] from SsDb resolve to [quarry_test].[users] without any per-connection SET equivalent (SQL Server has no search_path). - DisposeAsync() ROLLBACKs and closes the connection back to the pool. - Near-zero per-test overhead. Tests that need their own schema — migration runner tests, transaction- behavior tests, anything that issues its own BEGIN/COMMIT — pass useOwnSsSchema: true to CreateAsync(). That path provisions a per-harness schema test_<guid> and a dedicated short-lived login (default schema points at the new schema), connects as that login with no transaction, and drops the schema/user/login on dispose. SQL Server has no DROP SCHEMA CASCADE; teardown drops tables explicitly first. SQL Server DDL port (per Quarry.Migration.SqlTypeMapper.MapSqlServer): - Primary keys use INT IDENTITY(1,1). Seed inserts toggle SET IDENTITY_INSERT ... ON/OFF to supply explicit IDs. SQL Server tracks the high-water mark internally, so no PG-style setval step. - Money / decimal columns use DECIMAL(18, 2). - bool columns use BIT (defaults are 1/0, not TRUE/FALSE). - DateTime → DATETIME2; DateTimeOffset → DATETIMEOFFSET. - Identifiers are square-bracket quoted to match the SqlServer manifest emission. - Computed column: AS (...) PERSISTED instead of GENERATED ... STORED. - FK constraints stay omitted (same SQLite-parity reasoning as PG). No generator changes are required for SQL Server: Microsoft.Data.SqlClient already accepts @pn SQL with @pn parameter names — the configuration Quarry's existing emit produces. The Phase 4 mirror should pass on the existing emit. No new test sites in this phase; existing 2997 baseline still passes. Phase 3 will introduce the first Ss-execute integration tests. * Add Ss-execute integration tests + fix OUTPUT-clause placement on SqlServer (#270) Phase 3 of the SQL Server execution-mirror work. New integration tests (5): - SqlServerIntegrationTests.EntityInsert_OnSqlServer: single-entity INSERT with OUTPUT INSERTED. Reads back via Where + Select projection. - SqlServerIntegrationTests.InsertBatch_OnSqlServer: multi-row batch insert via ExecuteNonQueryAsync. - SqlServerIntegrationTests.WhereInCollection_OnSqlServer: runtime- expanded IN clause via the BuildWantedIds method-call shape that defeats the generator's constant-inlining pass. - SqlServerMigrationRunnerTests.RunAsync_InsertsHistoryRow_OnSqlServer: end-to-end MigrationRunner regression test, symmetric to the PostgresMigrationRunnerTests guard. Generator fix surfaced by EntityInsert: RenderInsertSql / RenderBatchInsertSql were emitting the OUTPUT INSERTED.[Id] clause AFTER the VALUES clause for SqlServer, which is invalid SQL Server syntax ("Incorrect syntax near 'OUTPUT'"). The OUTPUT clause must precede VALUES. Fixed both render paths; for batch insert the OUTPUT is folded into the prefix and the trailing returning suffix is suppressed for SqlServer. Other dialects (RETURNING for SQLite/PG, ; SELECT LAST_INSERT_ID() for MySQL) are unchanged. Test assertions updated (18 cross-dialect insert sites across 5 files): PrepareIntegrationTests, CrossDialectBatchInsertTests, CrossDialectEnum Tests, CrossDialectInsertTests, CrossDialectTypeMappingTests. The quarry-manifest.sqlserver.md regenerates automatically. Harness wiring: the transactional-rollback path now uses raw `BEGIN TRANSACTION` / `ROLLBACK TRANSACTION` SQL commands instead of SqlConnection.BeginTransaction(), because SqlClient requires every SqlCommand to have its `Transaction` property assigned when an explicit SqlTransaction is open — and Quarry's QueryExecutor builds DbCommands generically. Server-side semantics are identical. MsSqlTestContainer also gained CreateEmptySchemaAsync and a dynamic DROP loop in DropOwnedSchemaAsync so migration-runner tests can drop their __quarry_migrations + demo tables on teardown without listing them explicitly. Tests: 2997 baseline + 4 new Ss-execute integration tests = 3001. All green. * Mirror Pg-execute coverage to Ss across 22 cross-dialect files (#270) Phase 4 of the SQL Server execution-mirror work. Adds parallel `await ss.Execute*Async(...)` blocks to every `await pg.Execute*Async(...)` site across 22 CrossDialect*Tests files: 259 new ss execute sites, mechanically mirroring the assertion shape of the PG counterpart. Helper rename: PgRowOrderExtensions → RowOrderExtensions. The SortedByAsync helper is now used by both Pg-execute and Ss-execute mirror sites — SQL Server has the same no-row-order-without-ORDER-BY rule as PostgreSQL. Documentation generalised to mention both providers. Special handling: - CrossDialectMiscTests, CrossDialectSelectTests, CrossDialectSubquery Tests: tests that destructured `(Lite, Pg, _, _)` were updated to `(Lite, Pg, _, Ss)` so the Ss variable is in scope for the new mirror block. - CrossDialectTypeMappingTests.RoundTrip_InsertThenSelect_Preserves MoneyValue: a parallel Ss.Accounts().Insert(...) setup was added so the Ss SELECT mirror has data to read. - CrossDialectCompositionTests.Join_Distinct_OrderBy_Limit: skipped on Ss with comment referencing #267 (same DISTINCT + ORDER BY rule that PG hits). - CrossDialectCteTests.Cte_TwoChainedWiths_DistinctDtos_CapturedParams: uses the same `cutoff` variable rename PR #266 applied for PG; the underlying chained-With dispatch bug (#268) is dialect-independent. Phase 4 surfaced 19 Ss-only failures awaiting Phase 5 triage (see workflow.md ## Decisions for the categorisation): - 7 SqlDateTime-overflow failures from `default(DateTime)` parameter binding. - 9 Int64-to-Int32 cast failures from SQL Server window functions returning BIGINT. - 3 case-sensitivity-collation failures from SQL Server's default case-insensitive collation. Tests: 2982 passing, 19 failing on Ss execution path. Fixes land in Phase 5 commits, one per category. * Triage: case-sensitive collation on Ss schema (#270) Phase 5a triage: makes the SQL Server harness's NVARCHAR columns use COLLATE SQL_Latin1_General_CP1_CS_AS (case-sensitive, accent-sensitive) instead of inheriting the container's default SQL_Latin1_General_CP1_CI_AS (case-INsensitive). Aligns string-comparison semantics with SQLite, PostgreSQL, and MySQL — all of which compare case-sensitively by default. Resolves three Ss-only failures from Phase 4: - CrossDialectCompositionTests.Where_ContainsRuntimeCollection - CrossDialectCompositionTests.Where_Any_And_All_MultipleSubqueries - CrossDialectCompositionTests.Join_Where_InClause Each asserted Count == 0 for lowercase-vs-PascalCase non-matches; SQL Server's default collation made those queries match the seed data (returning 3, 2, 3 rows respectively). The COLLATE override is applied at column declaration only — generated SQL is unchanged. Other tests that compare exact-case strings continue to work without modification. Tests: 19 failing → 16 failing (3 datetime/Int64 categories remaining). * Triage: replace default(DateTime) with valid literal in 7 insert tests (#270) Phase 5b triage. Microsoft.Data.SqlClient binds .NET DateTime parameters as SqlDbType.DateTime by default — that type's range is 1753-01-01 to 9999-12-31, but `default(DateTime)` is 0001-01-01, which produces a SqlDateTime overflow at parameter-binding time even though the DATETIME2 column would accept the value. Replaces `CreatedAt = default` / `OrderDate = default` with `new DateTime(2024, 1, 1)` in CrossDialectInsertTests and CrossDialectEnumTests (the seven tests that surfaced the failure on Ss execution). The replacement value works for all four dialects; the test's intent (Insert succeeds and returns an identity > 0) is preserved on Lite and Pg. Resolves seven Ss-only failures from Phase 4: - Insert_SingleUser - Insert_SingleOrder - ExecuteScalarAsync_SingleUser_ReturnsIdentity - ExecuteScalarAsync_SingleOrder_ReturnsIdentity - ExecuteNonQueryAsync_SingleUser - ExecuteNonQueryAsync_SingleOrder - Insert_WithEnumColumn The underlying generator behaviour (binding DateTime as DATETIME on Ss) is preserved — the appropriate generator-side fix (forcing SqlDbType.DateTime2 on emitted parameters when the dialect is SqlServer) is out of scope for this PR. The new test values stay within the DATETIME range so this PR's tests remain robust against that fix landing later. Tests: 16 failing → 9 failing (only the Int64-vs-Int32 window-function category remains). * Triage: skip Ss execute on 9 window-function sites pending #274 (#270) Phase 5c triage. SQL Server returns ROW_NUMBER, DENSE_RANK, NTILE, COUNT-over-partition, etc. as BIGINT. Microsoft.Data.SqlClient.SqlData Reader.GetInt32 does not auto-narrow from BIGINT, so the generator- emitted reader fails with InvalidCastException when projecting a window-function result into an int-typed tuple element. Npgsql narrows silently, which is why PG passes the same projection. The fix belongs in the generator (cast-in-SQL on Ss, or read-with- GetInt64 on Ss). Filed as a separate issue (#274) to keep this PR scope-bounded. For the nine affected tests, the Ss-execute block is replaced with a comment referencing #274. The SQL-string emit assertion (in QueryTestHarness.AssertDialects above each block) still covers the generator's Ss output, so the emit-side regression guard is preserved even while the runtime side is unverified. Affected sites: - CrossDialectWindowFunctionTests: - WindowFunction_RowNumber_OrderBy - WindowFunction_DenseRank_OrderByDescending - WindowFunction_Joined_RowNumber - WindowFunction_WithWhereClause - WindowFunction_MixedTypePartitionBy - WindowFunction_Ntile_ConstVariable - WindowFunction_Ntile_Variable - CrossDialectSetOperationTests: - UnionAll_WithVariableWindowFunctionArg - UnionAll_WithVariableWindowFunctionArg_ParamOffset Tests: 9 failing → 0 failing. Suite is fully green: 3001 / 3001. * REMEDIATE: address review findings (#270) Addresses six A-classified review findings (one was documentation-only, folded into the PR body): 1. Mirror two missed pg-execute sites in CrossDialectSchemaTests.cs (Select_SingleColumn, Delete_All_NoWhereClause). The first site also gains the missing `.SortedByAsync(s => s)` to align with the PR #266 row-order-flake guard pattern; lt/pg/ss results all sort by the single-column UserName ('Alice' first), preserving the assertion. 2. Tighten MsSqlTestContainer.IsDockerUnavailable: removed the null-forgiving `!` and the redundant `break` from the inner-exception walk; loop now reads as the idiomatic `for (var cur = ex; cur is not null; cur = cur.InnerException)` shape. 3. DropOwnedSchemaAsync now uses SqlConnection.ClearPool(probe) keyed by the per-harness user's connection string instead of SqlConnection.ClearAllPools() (which would evict every other live harness's pool entries). The probe is a non-opened SqlConnection — no authentication round-trip, just connection-string-based pool keying. 4. Tighten the baseline readiness probe: * Renamed SchemaHasUsersTableAsync → SchemaHasSeededTableAsync and pointed it at the LAST seeded table (`shipments`) with a row-count check. A partial-baseline (early tables created but seed never finished) now correctly fails the check. * Made `CREATE SCHEMA` idempotent via `IF NOT EXISTS (sys.schemas) EXEC('CREATE SCHEMA …')`. Recovery from a previously crashed process no longer fails with "schema already exists". * Added DropAllObjectsInSchemaAsync call before re-creating tables on the recovery path, so any leftover partial-state objects are purged. 5. Move Ss.Dispose() to AFTER the rollback in QueryTestHarness.DisposeAsync, mirroring the PG path's wrapper-after-rollback ordering. No functional change today (SsDb.Dispose only disposes the wrapper) but removes the footgun for any future change to SsDb.Dispose semantics. The OUTPUT-clause emit shape change finding (#27) is documentation-only; the PR body's Breaking Changes section calls it out explicitly. The plan-vs-reality finding (#1, #2) is partially addressed by mirroring the SchemaTests sites and noted in the PR body's site-count summary. Tests: 3001 / 3001 still green. * Add PR body and link in workflow.md (#270) * chore: remove session artifacts before merge
DJGosnell
added a commit
that referenced
this pull request
Apr 29, 2026
…st + production-path test (#274) Post-review remediation for review findings: #2 (A) — Gate refinement: tighten the flag-set criterion in ProjectionAnalyzer to exclude LAG/LEAD/FIRST_VALUE/LAST_VALUE explicitly. Previously the gate was 'HasOverClauseLambda && clrType == "int"', which would wrap a user- written 'Sql.Lag(o.IntCol, ...)' on Ss despite the documented exclusion. LAG/LEAD/FirstValue/LastValue inherit the source column's type, so on Ss they return INT (not BIGINT) when the column is int — the wrap was a defensive no-op functionally, but the manifest/emit would diverge from other dialects which the spec promised would not happen. New helper 'IsIntReturningWindowFunction' centralises the rule used at all 4 ProjectedColumn construction sites. #1 (B) — workflow.md Decisions updated to record that the production wrap lives in SqlAssembler.AppendProjectionColumnSql, not the originally-planned ReaderCodeGenerator.GenerateColumnList (the latter is unreferenced inside the generator). #3+#10 (B) — Added cross-dialect 'WindowFunction_Lag_IntColumn_NoCast' regression test verifying no CAST is emitted on any dialect when the window function is LAG over an int column. #8 (B) — Added two production-path integration tests in SqlServerWindowIntCastTests that drive SqlAssembler end-to-end via QueryTestHarness + Prepare().ToDiagnostics(). Class-level XML doc updated to document the two-layer test split (helpers + production path). Refs #274.
DJGosnell
added a commit
that referenced
this pull request
Apr 29, 2026
…ption (#287) * feat(generator): add RequiresSqlServerIntCast flag on ProjectedColumn (#274) Pure model change. Default false; no upstream sets the flag yet. Subsequent commits wire the flag into the column-list emitter and into the projection analyzer at window-function emit sites. Refs #274. * feat(generator): emit CAST(... AS INT) on Ss when column flagged (#274) GenerateColumnList and GenerateColumnNamesArray wrap the rendered SQL expression with CAST(... AS INT) when ProjectedColumn.RequiresSqlServerIntCast is set and the dialect is SqlServer. Other dialects emit unchanged SQL. No upstream call site sets the flag yet, so behaviour is unchanged in this commit; the next commit wires ProjectionAnalyzer to set the flag at window-function emit sites. Adds nine SqlServerWindowIntCastTests verifying the dialect-conditional wrap and the flag-off no-op on every dialect. Refs #274. * feat(generator): wrap int-typed window projections with CAST(... AS INT) on Ss (#274) ProjectionAnalyzer sets RequiresSqlServerIntCast on every ProjectedColumn that is built from a Sql.* invocation with an OVER lambda whose resolved ClrType is 'int'. Covered functions: ROW_NUMBER, RANK, DENSE_RANK, NTILE, COUNT/SUM/AVG/MIN/MAX OVER (when target type is int). LAG, LEAD, FirstValue, LastValue inherit the source column's type and are not flagged. The wrap itself is applied in SqlAssembler.AppendProjectionColumnSql - the production SELECT-clause emit path. Only fires when dialect == SqlServer; other dialects emit identical SQL to before. The corresponding wrap in ReaderCodeGenerator.GenerateColumnList / GenerateColumnNamesArray (added in the previous commit) covers the unit-test entry points and any external consumers; both helpers already appeared to be unreferenced inside the generator but are public API. Existing CrossDialectWindowFunctionTests / CrossDialectSetOperationTests will fail their Ss SQL-string assertions until the next commit updates the expected strings; manifest snapshot is regenerated in the same step. Refs #274. * test: update Ss SQL assertions and regenerate Ss manifest for CAST(... AS INT) wrap (#274) The generator now emits CAST(... AS INT) around int-typed window-function projections on SQL Server. This commit syncs the affected expectations: - 16 ss: assertions in CrossDialectWindowFunctionTests.cs (ROW_NUMBER, RANK, DENSE_RANK, NTILE, COUNT(*) OVER projections) - 2 ss: assertions in CrossDialectSetOperationTests.cs (UnionAll_WithVariableWindowFunctionArg* — NTILE) - src/Quarry.Tests/ManifestOutput/quarry-manifest.sqlserver.md (auto-regenerated by the build) Other dialect manifests (postgresql/mysql/sqlite) are byte-identical; only Ss SQL changes. Sum/Avg/Min/Max OVER on decimal columns are not flagged and emit unchanged on every dialect. Refs #274. * test: un-skip Ss execution for nine window-function tests after #274 fix With the CAST(... AS INT) wrap in place, SqlDataReader.GetInt32 succeeds for ROW_NUMBER/DENSE_RANK/NTILE projections on SQL Server. Removes the nine 'ss execution skipped — see #274' comments and adds matching ssResults assertions mirroring the existing Pg/My execution paths. Verified end-to-end against Testcontainers MsSql; full suite (3,373 tests across Quarry.Tests / Quarry.Analyzers.Tests / Quarry.Migration.Tests) green. Closes #274. * fix(generator): tighten window-function-int-cast gate; add LAG-int test + production-path test (#274) Post-review remediation for review findings: #2 (A) — Gate refinement: tighten the flag-set criterion in ProjectionAnalyzer to exclude LAG/LEAD/FIRST_VALUE/LAST_VALUE explicitly. Previously the gate was 'HasOverClauseLambda && clrType == "int"', which would wrap a user- written 'Sql.Lag(o.IntCol, ...)' on Ss despite the documented exclusion. LAG/LEAD/FirstValue/LastValue inherit the source column's type, so on Ss they return INT (not BIGINT) when the column is int — the wrap was a defensive no-op functionally, but the manifest/emit would diverge from other dialects which the spec promised would not happen. New helper 'IsIntReturningWindowFunction' centralises the rule used at all 4 ProjectedColumn construction sites. #1 (B) — workflow.md Decisions updated to record that the production wrap lives in SqlAssembler.AppendProjectionColumnSql, not the originally-planned ReaderCodeGenerator.GenerateColumnList (the latter is unreferenced inside the generator). #3+#10 (B) — Added cross-dialect 'WindowFunction_Lag_IntColumn_NoCast' regression test verifying no CAST is emitted on any dialect when the window function is LAG over an int column. #8 (B) — Added two production-path integration tests in SqlServerWindowIntCastTests that drive SqlAssembler end-to-end via QueryTestHarness + Prepare().ToDiagnostics(). Class-level XML doc updated to document the two-layer test split (helpers + production path). Refs #274. * docs(session): record issue #286 in review.md classifications * docs(session): record PR #287 in workflow.md state * test: regenerate manifests post-rebase for #274 + LAG-int regression test Rebase against origin/master (#284 landed) caused conflicts in all four dialect manifests because both sides added entries. Resolved by taking master's version then rebuilding to regenerate; result includes both master's additions (from #284) and this branch's #274 + LAG-int additions. Refs #274. * chore: remove session artifacts before merge
DJGosnell
added a commit
that referenced
this pull request
Apr 29, 2026
(A) findings — addressed in code: - Doc comments on `LikeExpr.NeedsEscape` and `EscapeLikeMetaChars` updated to flag dialect-aware render-time escaping (#1) - New `MaybeDoubleBackslashes` helper: prefix/suffix in non-folded LIKE concat path now defensively backslash-doubled for MySQL+default (#7) - Inline comments at both literal-pattern emission sites explain the single-quote-first / backslash-double-second order (#9) - `SqlDialectConfig.ParseAttribute` returns `(Config, Schema)` tuple in a single pass; `ContextParser` consumes the tuple (#28) - Code comment in `ParseAttribute` documents the SQLite default for missing `Dialect=` as intentional pre-existing behavior (#11) - All 11 method bodies in `SqlAssembler.cs` now uniformly use the `var dialect = config.Dialect;` shadow at entry (#25) - Extracted `IsDockerUnavailable`, `TableExistsAsync`, `ExecAsync` from the two MySQL test containers into shared `TestContainerHelpers.cs` (#26) - Added `<remarks>` footgun warning to the back-compat `Render(SqlDialect)` overload — silently defaults the carrier flag (#33) - Updated `MySqlBackslashEscapes` XML doc to make non-MySQL no-op behavior explicit; future QRY rule noted as follow-up (#37) (B) findings — added gap-filling tests: - `Contains_ParameterBound_BackslashEscapesTrue_NoDoubling` proves the parameter-bound LIKE path bypasses doubling (#20) - `Where_Contains_LiteralBackslash_DefaultMode_ReturnsMatch` integration test exercises the seeded-but-unqueried `"a\b"` row through the full pipeline against live default-mode MySQL (#21) - `Where_Contains_AnsiForm_NoBackslashEscapesSession_ReturnsMatch` integration test exercises the opt-out roundtrip with new `MyAnsiSessionDb` context against default-mode container with session-level `SET sql_mode = ...,NO_BACKSLASH_ESCAPES` (#2) All 3,384 tests pass.
DJGosnell
added a commit
that referenced
this pull request
May 6, 2026
…ves) Addresses the four A/B findings from the REVIEW pass: - Finding #1 (B): adds `Select_ProjectionMixedNestingDepths_OrderTotalAndItemTotal` to `CrossDialectNestedSubqueryTests` — closes the deep-projection-side gap that the sibling 1-level test left uncovered. The plan's 3-level Sum/Sum/Count variant exposed a generator projection-type resolver bug (nested int aggregates resolve as decimal, CS9144); tracked in #294 and worked around by keeping the new test on decimal-typed Sums. - Findings #3 + #9 (B): adds `QRY075_UpdateSetAction_AssignToComputedColumn_Reports` and `QRY075_UpdateSetAction_AssignToWritableColumn_DoesNotReport` to `ComputedColumnDiagnosticTests` — provides the positive firing assertion that was missing after 1abdd63 trimmed the dead typed-lambda tests. - Finding #7 (A): renames `ToAsyncEnumerable_BreakEarly_StopsAfterFirstRow` to `ToAsyncEnumerable_BreakAfterFirst_YieldsOrderedFirstRow` and rewrites the comment to admit the assertion is behavioral, not a streaming-vs- buffering proof. - Finding #8 (A): tightens the `MultiContextPerFileTests` class docstring to state the test only proves file separation; carrier-name non-collision is attributed to C# `file`-scoped accessibility (language guarantee, not a generator behavior the test observes). Manifest snapshots regenerate to pick up the new projection query shape.
DJGosnell
added a commit
that referenced
this pull request
May 19, 2026
Introduces TypeClassification.UnresolvedTypeMarker = "?" as the canonical sentinel for an unresolved aggregate CLR type produced by Stage 1 syntax-only analysis. Replaces the bare "object" magic-string at the 8 Sum/Avg call sites in ProjectionAnalyzer (regular, joined, window, and joined-window aggregate paths) with the named constant, removing the ambiguity between the legitimate "object" CLR type and an unresolved-pending-enrichment marker. Behavior preserving — both IsUnresolvedTypeName helpers already recognize "?". Min/Max defaults remain at "object" (broader follow-up #1 in plan). Pairs with the earlier reorder/gate of ResolveAggregateClrType (WIP 892312d) to close Phase 1 of benchmark-double-migration. All 5 AggregateTypeResolutionTests pass; full suite green (146 + 201 + 3143 = 3490 passed, 0 failed).
DJGosnell
added a commit
that referenced
this pull request
May 21, 2026
…pe fix (#297) * [WIP] Quarry.Generator: fix aggregate CLR-type resolution (partial) + session Phase 1 of benchmark-double-migration, suspended before the typed-marker rename per user pushback on the "object" sentinel. What is in this commit: - ProjectionAnalyzer.ResolveAggregateClrType reordered to consult the schema-driven column lookup first, then SemanticModel argument type, then a gated SemanticModel invocation-return-type fallback. - 6 Sum/Avg call sites changed from the bogus "decimal" default to the interim "object" sentinel so ChainAnalyzer.BuildProjection's enrichment pass converts the unresolved type into the real column type. - 5 new tests in AggregateTypeResolutionTests.cs covering Sum over double/decimal/int/long columns and Avg over double — all passing. - Full suite remains green at 3482/3482 (baseline 3477 + 5 new). What remains in Phase 1: - Replace the bare "object" sentinel at the 6 aggregate call sites with a named TypeClassification.UnresolvedTypeMarker constant ("?"), already recognized by both IsUnresolvedTypeName helpers. Rename refactor only; behavior unchanged. See _sessions/benchmark-double-migration/ for the full workflow state. * [WIP] session: record WIP commit hash 892312d and session log entry * Quarry.Generator: complete Phase 1 — typed UnresolvedTypeMarker sentinel Introduces TypeClassification.UnresolvedTypeMarker = "?" as the canonical sentinel for an unresolved aggregate CLR type produced by Stage 1 syntax-only analysis. Replaces the bare "object" magic-string at the 8 Sum/Avg call sites in ProjectionAnalyzer (regular, joined, window, and joined-window aggregate paths) with the named constant, removing the ambiguity between the legitimate "object" CLR type and an unresolved-pending-enrichment marker. Behavior preserving — both IsUnresolvedTypeName helpers already recognize "?". Min/Max defaults remain at "object" (broader follow-up #1 in plan). Pairs with the earlier reorder/gate of ResolveAggregateClrType (WIP 892312d) to close Phase 1 of benchmark-double-migration. All 5 AggregateTypeResolutionTests pass; full suite green (146 + 201 + 3143 = 3490 passed, 0 failed). * Benchmarks: migrate Total/UnitPrice/LineTotal from decimal to double Phase 2 of benchmark-double-migration. Switches the money-shaped columns on OrderSchema/OrderItemSchema and their EF/DTO mirrors from decimal to double, and updates DatabaseSetup seed literals (10.0m/1.5m/5.0m/2.5m → 10.0/1.5/5.0/2.5) accordingly. The DapperOrderLagDto workaround class is removed; the regular OrderLagDto now uses double/double? so Dapper no longer needs a parallel type. This removes the SQLite GetDecimal(string-parse) per-cell tax from every reader on the benchmark hot path, so the inter-library comparison measures library overhead rather than a driver implementation choice. Quarry.Benchmarks does NOT compile in isolation after this commit (the reader bodies still call GetDecimal/ExecuteScalarAsync<decimal> and three benchmark files remain parked as `.cs.disabled`). Phase 3 restores compilation. Quarry.Tests builds and the full test suite still passes. * Benchmarks: update readers and Aggregate/Avg signatures to double Phase 3 of benchmark-double-migration. Restores compilation of Quarry.Benchmarks after the Phase 2 schema/DTO migration: - Replace `reader.GetDecimal(1)` with `reader.GetDouble(1)` across 8 reader benchmarks (CteSimple, CteMulti, CteProjection, ComplexJoinFilterPaginate, JoinInner, JoinThreeTable, WindowLag, WindowRunningSum) — 13 call sites total. - WindowLagBenchmarks.Dapper_Lag now uses the regular OrderLagDto; the DapperOrderLagDto workaround class was removed in Phase 2. - AggregateSumBenchmarks / AggregateAvgBenchmarks switched their Raw/Dapper/EfCore/Quarry/SqlKata methods from `Task<decimal>` / `ExecuteScalarAsync<decimal>` / `Convert.ToDecimal` to `<double>` / `Convert.ToDouble`. Quarry.Benchmarks builds clean. * Benchmarks: remove obsolete GetDecimal documentation Phase 4 of benchmark-double-migration. With the schema now using `double` for Total/UnitPrice/LineTotal, the GetDecimal string-parse cost is no longer on the benchmark hot path and the documentation explaining it is stale. Drops: - The canonical 22-line NOTE block at the top of CteSimpleBenchmarks.cs that documented the SqliteValueReader.GetDecimal implementation, Dapper's IL-emitted indexer path, and Quarry's deliberate refusal to use the (decimal)GetDouble trick. - The 7 cross-reference comment blocks in ComplexJoinFilterPaginate, CteMulti, CteProjection, JoinInner, JoinThreeTable, WindowLag, and WindowRunningSum that pointed at the canonical block. The rationale lives in PR and commit history for anyone who needs it. * Workflow: mark Phase 5 complete, transition to REVIEW Full test suite green in Release (3490 / 3490 passed). Benchmark smoke run executed 30 representative benchmarks across CteSimple, AggregateSum, and WindowLag end-to-end with no failures; Quarry tracks the hand-rolled Raw baseline within ~1% on WindowLag (~132µs each), matching the empirical numbers cited in the problem statement. All 5 implementation phases of benchmark-double-migration are complete; advancing to REVIEW. * REMEDIATE: extend Stage 4 enrichment to joined scalar aggregates Addresses review findings #14 (A, low) and #10 (B, medium): - TypeClassification.UnresolvedTypeMarker: public const → internal const (matches the class's accessibility). - Add 3 unit tests for joined-aggregate, single-entity window-aggregate, and joined-window-aggregate paths to AggregateTypeResolutionTests. Writing the joined tests surfaced a latent bug deeper than the original fix scope: ProjectionAnalyzer.AnalyzeJoinedInvocation (the entry point for `(u, o) => Sql.Sum(o.Total)` scalar joined projections) was constructing the aggregate ProjectedColumn without setting TableAlias. Stage 4 ChainAnalyzer.BuildProjection's aggregate enrichment then called TryResolveAggregateTypeFromSql with a null tableAlias, the alias-keyed lookup in perAliasLookup fell through, and the unresolved marker leaked all the way to the carrier (IJoinedQueryBuilder<User, Order, ?>) and reader Func type — uncompilable. The sibling ResolveJoinedAggregate (used for tuple-element joined projections) already extracts and sets TableAlias correctly. The fix mirrors that logic in AnalyzeJoinedInvocation — pull the alias from the first column argument (o.Total → "t1") and pass it through to the ProjectedColumn constructor. Behavior preserving for all previously-working paths. Joined scalar aggregates over non-decimal columns now compile and produce correctly- typed readers. Full suite: 201 + 146 + 3146 = 3493 / 3493 passed. See _sessions/benchmark-double-migration/review.md and workflow.md decision dated 2026-05-19 for full context. * chore: remove session artifacts before merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add per-project README files for NuGet packages and include Quarry.Analyzers.CodeFixes in all package tables. Fix Directory.Build.targets to strip images from each project's own README during pack.
Summary
PackageReadmeSourcepropertyIsPackable=trueto Generator, Analyzers, and Analyzers.CodeFixes csproj filesReason for Change
NuGet packages were either missing README files entirely (Generator, Analyzers, CodeFixes) or embedding the root README regardless of project. The new CodeFixes package had no listing in any packages table.
Impact
NuGet package pages for all five packages will now display their own project-specific README with the Quarry logo header. The
StripImgTagsMSBuild task strips<img>tags before embedding so NuGet renders cleanly.Migration Steps
None required.
Performance Considerations
None. Changes are build-time/packaging only.
Security Considerations
None.
Breaking Changes