Benchmark suite migration: decimal -> double + generator aggregate-type fix

[DJGosnell]

Summary

Two coupled changes:

1. Benchmark suite migration — Total, UnitPrice, LineTotal columns switched from decimal to double to remove Microsoft.Data.Sqlite.GetDecimal's per-cell decimal.Parse(GetString(ordinal), ...) string-parse tax from the benchmark hot path, so the inter-library comparison reflects library overhead rather than a driver implementation choice.
2. Generator bug fixes in ProjectionAnalyzer — surfaced by the migration: aggregate CLR-type resolution silently miscompiled Sql.Sum/Sql.Avg over any non-decimal column. Fix has two parts: a reorder + typed-marker change to the aggregate type resolver, plus extending Stage 4 enrichment to reach joined scalar aggregates (latent bug uncovered during REMEDIATE).

Reason for Change

Empirical measurements on CteSimpleBenchmarks and WindowLagBenchmarks showed Dapper appearing 19–30% faster than the hand-rolled Raw baseline on decimal-column workloads. Tracing identified the cause: Microsoft.Data.Sqlite.SqliteValueReader.GetDecimal is implemented as decimal.Parse(GetString(ordinal), NumberStyles.Number | AllowExponent, InvariantCulture) — a string allocation + culture-aware parse per cell. Raw / Quarry / SqlKata all hit this path; Dapper's IL-emitted deserializer sidesteps it by reading through the DbDataReader indexer (boxed double → unbox.any → (decimal)(double) conversion). The benchmark numbers were measuring a SQLite driver characteristic, not library overhead.

Migrating to double removes the slow path. Empirical result: Quarry tracks Raw within ~0.5% (~132µs vs ~132µs on WindowLag), Dapper drops from "fastest" to ~13–31% slower than Raw (boxing tax now dominates), and per-row allocations drop ~35% for Raw/Quarry/SqlKata.

The migration exposed a latent generator bug that had to be fixed in the same branch so the benchmark project could compile against Col<double> columns.

Impact

• Library users: Generator fix only changes behavior when prior behavior was demonstrably wrong (aggregate over non-decimal columns silently produced wrong CLR types). Schemas aggregating over Col<decimal> continue to resolve to decimal. No silent behavior change for previously-working code.
• Benchmark consumers: Benchmark numbers are now an apples-to-apples comparison of library overhead. The decimal → double schema change is internal to the Quarry.Benchmarks console project (no package surface).

Plan items implemented as specified

• Phase 1 — Generator fix: Reorder ResolveAggregateClrType to consult column lookup first, then SemanticModel argument type, then a gated invocation-return-type fallback. Introduce TypeClassification.UnresolvedTypeMarker = "?" and migrate the 8 Sum/Avg call sites from a "decimal" default to the typed sentinel. Min/Max defaults intentionally left at "object" (deferred to follow-up).
• Phase 2 — Schema/entity/DTO/seed migration: OrderSchema.Total, OrderItemSchema.UnitPrice / LineTotal → Col<double>; EF entities and DTOs mirrored; DapperOrderLagDto workaround class removed; DatabaseSetup seed literals updated to double.
• Phase 3 — Reader call updates: reader.GetDecimal(N) → reader.GetDouble(N) across 8 benchmark files; AggregateSum/AggregateAvg benchmark methods switched from Task<decimal> / ExecuteScalarAsync<decimal> / Convert.ToDecimal to <double> / Convert.ToDouble.
• Phase 4 — Comment cleanup: Dropped the 22-line canonical NOTE block in CteSimpleBenchmarks.cs and 7 cross-reference comment blocks in other benchmarks.
• Phase 5 — Validation: Full Release-mode suite green; benchmark dry-run smoke executed 30 benchmarks across CteSimple / AggregateSum / WindowLag end-to-end with no failures.

Deviations from plan implemented

• Phase 1 root-cause depth: The original DESIGN diagnosis was "reorder ResolveAggregateClrType's try-priority". Implementation showed the reorder alone wasn't sufficient — in Stage 1 (syntax-only discovery) columnLookup is intentionally empty, so the column-lookup branch can never succeed at that point. The actual mechanism is two-stage: Stage 1 returns an unresolved-type sentinel and Stage 4 (ChainAnalyzer.BuildProjection) walks aggregate columns whose ClrType is unresolved and enriches them via TryResolveAggregateTypeFromSql. Min/Max already used "object" (recognized by IsUnresolvedTypeName) and worked via enrichment; Sum/Avg defaulted to "decimal" which Stage 4 treats as a resolved type, skipping enrichment. The minimal correctness fix was to change Sum/Avg defaults from "decimal" to an unresolved sentinel.
• Typed sentinel: Per user pushback, the bare "object" magic-string sentinel was replaced with TypeClassification.UnresolvedTypeMarker = "?" (a named constant already partially recognized by both IsUnresolvedTypeName helpers). Reorder/gate fix kept as future-proofing against (a) joined contexts where column lookup IS populated and (b) Roslyn behavior changes in overload-resolution heuristics against Error-typed arguments.

Gaps in original plan implemented

• REMEDIATE — Stage 4 enrichment for joined scalar aggregates: Writing the joined-aggregate / joined-window-aggregate unit tests (review finding Feat: Carrier class optimization for PrebuiltDispatch chains #10) surfaced a deeper latent bug: ProjectionAnalyzer.AnalyzeJoinedInvocation (entry point for (u, o) => Sql.Sum(o.Total) scalar joined projections) was constructing the aggregate ProjectedColumn without setting TableAlias. Stage 4 enrichment then called TryResolveAggregateTypeFromSql with a null tableAlias, the alias-keyed lookup in perAliasLookup fell through, and the unresolved marker leaked all the way through to the carrier interface (IJoinedQueryBuilder<User, Order, ?>) and reader Func type — uncompilable. The sibling ResolveJoinedAggregate (tuple-element handler) already extracted the alias correctly. The fix mirrors that logic in AnalyzeJoinedInvocation. With the prior "object" default the bug was masked (it emitted Func<object> and (object)r.GetValue(0) — compilable but semantically wrong); the typed-marker rename made the latent bug observable.

Migration Steps

None — internal-only change. The benchmark project rebuilds cleanly; no downstream consumers exist (Quarry.Benchmarks.csproj is OutputType=Exe with no PackageId).

Performance Considerations

Benchmark numbers measured pre-merge (Quarry.Benchmarks --filter "*WindowLagBenchmarks*", default job):

• Quarry 132.0µs / Raw 132.8µs / Dapper 150.4µs / SqlKata 163.0µs / EfCore 166.2µs. Quarry tracks Raw within ~1%.
• Per-row allocations drop ~35% for Raw / Quarry / SqlKata (per-row string allocations from GetDecimal disappear).

Security Considerations

None. Generator fix is a strict bug fix; benchmark migration is internal-only.

Breaking Changes

• Consumer-facing: None. Col<decimal> remains supported and used in ~30 other files (Samples, tests, GeneratorHarness, docs). The generator fix only changes behavior when prior behavior was demonstrably wrong (CS9144 signature mismatch on aggregates over non-decimal columns). TypeClassification.UnresolvedTypeMarker is internal const on an internal static class — not part of any public surface.
• Internal: DapperOrderLagDto workaround class removed (only used inside the benchmark project; superseded by OrderLagDto now using double / double?). Schema shape changes on OrderSchema.Total / OrderItemSchema.UnitPrice / OrderItemSchema.LineTotal from Col<decimal> to Col<double> are local to the benchmark project.

Known follow-ups (out of scope)

Tracked in _sessions/benchmark-double-migration/plan.md "Known follow-ups":

1. Migrate the rest of the "object"-as-sentinel usage in ProjectionAnalyzer (Min/Max defaults) to TypeClassification.UnresolvedTypeMarker.
2. Move from string sentinels to a type-safe ResolvedClrType discriminated union.
3. Cross-dialect aggregate-type test fixtures with Col<double> columns.
4. Pass EntityRegistry into Stage 1 syntax-only analysis so the sentinel becomes unnecessary entirely.

Test plan

• Full suite: 201 + 146 + 3146 = 3493 / 3493 passed (Release mode green; Debug mode green)
• AggregateTypeResolutionTests (8 NUnit tests covering Sum / Avg over Col<double> / Col<decimal> / Col<int> / Col<long>, plus joined-aggregate, single-entity window-aggregate, and joined-window-aggregate paths)
• Benchmark project compiles clean in Debug and Release
• Benchmark dry-run smoke (30 benchmarks across CteSimple / AggregateSum / WindowLag) executed end-to-end without errors
• No GetDecimal / decimal / DapperOrderLagDto references remain anywhere in src/Quarry.Benchmarks