Commit 6225678
authored
Benchmark suite migration: decimal -> double + generator aggregate-type 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 merge1 parent 08d8323 commit 6225678
18 files changed
Lines changed: 481 additions & 131 deletions
File tree
- src
- Quarry.Benchmarks
- Benchmarks
- Infrastructure
- Schemas
- Quarry.Generator
- Projection
- Utilities
- Quarry.Tests/Generation
Lines changed: 9 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
| 34 | + | |
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
| 38 | + | |
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
Lines changed: 9 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
| 34 | + | |
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
| 38 | + | |
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
31 | 29 | | |
32 | 30 | | |
33 | 31 | | |
34 | | - | |
| 32 | + | |
35 | 33 | | |
36 | 34 | | |
37 | 35 | | |
| |||
105 | 103 | | |
106 | 104 | | |
107 | 105 | | |
108 | | - | |
| 106 | + | |
109 | 107 | | |
110 | 108 | | |
111 | 109 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
37 | 35 | | |
38 | 36 | | |
39 | 37 | | |
40 | | - | |
| 38 | + | |
41 | 39 | | |
42 | 40 | | |
43 | 41 | | |
| |||
84 | 82 | | |
85 | 83 | | |
86 | 84 | | |
87 | | - | |
| 85 | + | |
88 | 86 | | |
89 | 87 | | |
90 | 88 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
33 | 31 | | |
34 | 32 | | |
35 | 33 | | |
36 | | - | |
| 34 | + | |
37 | 35 | | |
38 | 36 | | |
39 | 37 | | |
| |||
86 | 84 | | |
87 | 85 | | |
88 | 86 | | |
89 | | - | |
| 87 | + | |
90 | 88 | | |
91 | 89 | | |
92 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | 12 | | |
36 | 13 | | |
37 | 14 | | |
| |||
54 | 31 | | |
55 | 32 | | |
56 | 33 | | |
57 | | - | |
| 34 | + | |
58 | 35 | | |
59 | 36 | | |
60 | 37 | | |
| |||
101 | 78 | | |
102 | 79 | | |
103 | 80 | | |
104 | | - | |
| 81 | + | |
105 | 82 | | |
106 | 83 | | |
107 | 84 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
25 | 23 | | |
26 | 24 | | |
27 | 25 | | |
28 | | - | |
| 26 | + | |
29 | 27 | | |
30 | 28 | | |
31 | 29 | | |
| |||
85 | 83 | | |
86 | 84 | | |
87 | 85 | | |
88 | | - | |
| 86 | + | |
89 | 87 | | |
90 | 88 | | |
91 | 89 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
30 | 28 | | |
31 | 29 | | |
32 | 30 | | |
33 | | - | |
| 31 | + | |
34 | 32 | | |
35 | 33 | | |
36 | 34 | | |
| |||
100 | 98 | | |
101 | 99 | | |
102 | 100 | | |
103 | | - | |
| 101 | + | |
104 | 102 | | |
105 | 103 | | |
106 | 104 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
30 | 28 | | |
31 | 29 | | |
32 | 30 | | |
33 | | - | |
34 | | - | |
| 31 | + | |
| 32 | + | |
35 | 33 | | |
36 | 34 | | |
37 | 35 | | |
38 | 36 | | |
39 | 37 | | |
40 | 38 | | |
41 | | - | |
| 39 | + | |
42 | 40 | | |
43 | | - | |
| 41 | + | |
44 | 42 | | |
45 | 43 | | |
46 | 44 | | |
| |||
80 | 78 | | |
81 | 79 | | |
82 | 80 | | |
83 | | - | |
84 | | - | |
| 81 | + | |
| 82 | + | |
85 | 83 | | |
86 | 84 | | |
87 | 85 | | |
| |||
Lines changed: 4 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
30 | 28 | | |
31 | 29 | | |
32 | 30 | | |
33 | | - | |
34 | | - | |
| 31 | + | |
| 32 | + | |
35 | 33 | | |
36 | 34 | | |
37 | 35 | | |
| |||
80 | 78 | | |
81 | 79 | | |
82 | 80 | | |
83 | | - | |
84 | | - | |
| 81 | + | |
| 82 | + | |
85 | 83 | | |
86 | 84 | | |
87 | 85 | | |
| |||
0 commit comments