Skip to content

Commit e939a7a

Browse files
tensordreamsaqrln
authored andcommitted
refactor(sql-orm): address PR review — AST-level adapter docs, leaner nested-include helper
- Rename `buildNestedIncludeArtifacts` -> `buildNestedIncludeProjections` and return the projection array directly instead of wrapping it in an object. - Postgres/SQLite READMEs: describe what the adapter actually renders at the AST level (JSON aggregation functions, scalar subqueries, LATERAL joins) and drop all references to `.include(...)`, lanes, and the ORM client — the adapter does not know which lane produced the AST. - include.test.ts: drop transient ticket IDs from test names/comments. - nested-includes-strategy.test.ts: assert `ast.joins` is exactly `[]` (strictly stronger than "no lateral join") in the lateral-flag-inert guard. Refs: TML-2729 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
1 parent ee5c9f3 commit e939a7a

5 files changed

Lines changed: 37 additions & 48 deletions

File tree

packages/3-extensions/sql-orm-client/src/query-plan-select.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -271,25 +271,21 @@ function wrapWithRowNumberDedup(options: {
271271
}
272272

273273
/**
274-
* Recursively build the projection artifacts for the nested includes
275-
* attached to a child SELECT. Used by `buildIncludeChildRowsSelect` to
276-
* wire depth-2+ aggregates into the inner SELECT at each level.
274+
* Recursively build the correlated-subquery projections for the nested
275+
* includes attached to a child SELECT. Used by `buildIncludeChildRowsSelect`
276+
* to wire depth-2+ aggregates into the inner SELECT at each level.
277277
*
278278
* Each nested include contributes a single projection item whose
279-
* expression is a correlated subquery; no joins are produced.
279+
* expression is a correlated subquery.
280280
*/
281-
function buildNestedIncludeArtifacts(
281+
function buildNestedIncludeProjections(
282282
contract: Contract<SqlStorage>,
283283
parentTableRef: string,
284284
includes: readonly IncludeExpr[],
285-
): {
286-
readonly projections: ReadonlyArray<ProjectionItem>;
287-
} {
288-
const projections = includes.map(
285+
): ReadonlyArray<ProjectionItem> {
286+
return includes.map(
289287
(nested) => buildCorrelatedIncludeProjection(contract, parentTableRef, nested).projection,
290288
);
291-
292-
return { projections };
293289
}
294290

295291
function buildIncludeChildRowsSelect(
@@ -373,7 +369,7 @@ function buildIncludeChildRowsSelect(
373369
// projection. The nested aggregates are attached to *this* child
374370
// SELECT, so they correlate against `childTableRef` — which may itself
375371
// be an alias if the relation is self-referential.
376-
const { projections: nestedProjections } = buildNestedIncludeArtifacts(
372+
const nestedProjections = buildNestedIncludeProjections(
377373
contract,
378374
childTableRef,
379375
childState.includes,
@@ -564,7 +560,7 @@ function buildDistinctNonLeafChildRowsSelect(options: {
564560
childState.selectedFields,
565561
distinctAlias,
566562
);
567-
const { projections: outerNestedProjections } = buildNestedIncludeArtifacts(
563+
const outerNestedProjections = buildNestedIncludeProjections(
568564
contract,
569565
distinctAlias,
570566
childState.includes,

packages/3-targets/6-adapters/postgres/README.md

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Provide PostgreSQL-specific adapter implementation, codecs, and capabilities. En
2020

2121
- **Adapter Implementation**: Implement `Adapter` SPI for PostgreSQL
2222
- Lower SQL ASTs to PostgreSQL dialect SQL
23-
- Render `.include(...)` as a correlated subquery with `json_agg` for nested array includes
23+
- Render JSON aggregation (`json_agg`, `json_build_object`) and scalar subqueries
2424
- Advertise PostgreSQL capabilities (`lateral`, `jsonAgg`)
2525
- Normalize PostgreSQL EXPLAIN output
2626
- Map PostgreSQL errors to `RuntimeError` envelope
@@ -89,8 +89,8 @@ flowchart TD
8989
**Adapter (`adapter.ts`)**
9090
- Main adapter implementation
9191
- Lowers SQL ASTs to PostgreSQL SQL
92-
- Renders joins (INNER, LEFT, RIGHT, FULL) with ON conditions
93-
- Renders `.include(...)` as a correlated subquery with `json_agg` for nested array includes
92+
- Renders joins (INNER, LEFT, RIGHT, FULL, LATERAL) with ON conditions
93+
- Renders JSON aggregation (`json_agg`, `json_build_object`) and scalar subqueries
9494
- Renders DML operations (INSERT, UPDATE, DELETE) with RETURNING clauses
9595
- Advertises PostgreSQL capabilities (`lateral`, `jsonAgg`, `returning`)
9696
- Maps PostgreSQL errors to `RuntimeError`
@@ -191,8 +191,8 @@ The adapter declares the following PostgreSQL capabilities:
191191

192192
- **`orderBy: true`** - Supports ORDER BY clauses
193193
- **`limit: true`** - Supports LIMIT clauses
194-
- **`lateral: true`** - Supports LATERAL joins (used by the SQL-builder `lateralJoin()` DSL)
195-
- **`jsonAgg: true`** - Supports JSON aggregation functions (`json_agg`) used to lower `.include(...)` to correlated subqueries
194+
- **`lateral: true`** - Supports LATERAL joins
195+
- **`jsonAgg: true`** - Supports JSON aggregation functions (`json_agg`)
196196
- **`returning: true`** - Supports RETURNING clauses for DML operations (INSERT, UPDATE, DELETE)
197197
- **`sql.enums: true`** - Supports contract-defined enum storage types
198198

@@ -205,17 +205,13 @@ The capabilities on the descriptor must match the capabilities in code. If they
205205

206206
See `docs/reference/capabilities.md` and `docs/architecture docs/subsystems/5. Adapters & Targets.md` for details.
207207

208-
## Include Support
208+
## JSON Aggregation
209209

210-
The adapter lowers `.include(...)` for nested array includes to a correlated subquery in the SELECT list, using `json_agg`:
210+
The renderer lowers JSON-aggregation AST nodes to PostgreSQL's `json_agg`:
211211

212-
**Lowering Strategy:**
213-
- Renders each `.include(...)` as a correlated subquery that uses `json_agg(json_build_object(...))` to aggregate child rows into a JSON array
214-
- The ON condition from the include becomes the WHERE clause of the correlated subquery, referencing the outer row
215-
- When both `ORDER BY` and `LIMIT` are present, wraps the child query in an inner SELECT that projects individual columns with aliases, then uses `json_agg(row_to_json(sub.*))` on the result
216-
217-
**Capabilities Required:**
218-
- `jsonAgg: true` - Enables `json_agg` function support
212+
- `json_agg(json_build_object(...))` aggregates a row set into a JSON array of objects
213+
- A scalar subquery (`SubqueryExpr`) in the SELECT list correlates against the outer row through its WHERE clause
214+
- When the subquery carries an inner `ORDER BY` and `LIMIT`, its rows are wrapped in an inner SELECT, then aggregated with `json_agg(row_to_json(sub.*))`
219215

220216
**Example SQL Output:**
221217
```sql
@@ -227,8 +223,6 @@ SELECT "user"."id" AS "id", (
227223
FROM "user"
228224
```
229225

230-
> LATERAL emission is retained in the renderer (and the `lateral` capability stays advertised) for the public SQL-builder `lateralJoin()` DSL; the ORM include path no longer uses it.
231-
232226
## DML Operations with RETURNING
233227

234228
The adapter supports RETURNING clauses for DML operations (INSERT, UPDATE, DELETE), allowing you to return affected rows:

packages/3-targets/6-adapters/sqlite/README.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Provide SQLite-specific adapter implementation, codecs, and capabilities. Enable
2020

2121
- **Adapter Implementation**: Implement `Adapter` SPI for SQLite
2222
- Lower SQL ASTs to SQLite dialect SQL
23-
- Render `.include(...)` as correlated subquery with `json_group_array(json_object(...))` for nested array includes
23+
- Render JSON aggregation (`json_group_array`, `json_object`) and scalar subqueries
2424
- Advertise SQLite capabilities (`jsonAgg`, `returning`; no `lateral`, no `enums`)
2525
- Provide target-specific marker SQL via `readMarkerStatement()` on `AdapterProfile`
2626
- Map SQLite errors to `RuntimeError` envelope
@@ -91,7 +91,7 @@ flowchart TD
9191
- Main adapter implementation
9292
- Lowers SQL ASTs to SQLite SQL with `?` positional parameters
9393
- Renders joins (INNER, LEFT, RIGHT, FULL) with ON conditions
94-
- Renders `.include(...)` as correlated subquery with `json_group_array(json_object(...))` for nested array includes
94+
- Renders JSON aggregation (`json_group_array`, `json_object`) and scalar subqueries
9595
- Renders DML operations (INSERT, UPDATE, DELETE) with RETURNING clauses
9696
- Renders ON CONFLICT (DO NOTHING / DO UPDATE SET) for upserts
9797
- Uses `CAST(expr AS type)` instead of Postgres `::type` syntax
@@ -148,18 +148,18 @@ The adapter declares the following SQLite capabilities:
148148

149149
- **`sql.orderBy: true`** -- Supports ORDER BY clauses
150150
- **`sql.limit: true`** -- Supports LIMIT clauses
151-
- **`sql.lateral: false`** -- No LATERAL join support; `.include(...)` uses correlated subqueries
152-
- **`sql.jsonAgg: true`** -- Supports JSON aggregation via `json_group_array()` for `.include(...)`
151+
- **`sql.lateral: false`** -- No LATERAL join support
152+
- **`sql.jsonAgg: true`** -- Supports JSON aggregation via `json_group_array()`
153153
- **`sql.returning: true`** -- Supports RETURNING clauses for DML operations (SQLite 3.35+)
154154
- **`sql.enums: false`** -- No native enum support
155155

156-
## Include Support
156+
## JSON Aggregation
157157

158-
The adapter lowers `.include(...)` for nested array includes using SQLite's `json_group_array()` and `json_object()`:
158+
The renderer lowers JSON-aggregation AST nodes using SQLite's `json_group_array()` and `json_object()`:
159159

160-
**Lowering Strategy:**
161-
- Renders `.include(...)` as a correlated subquery with `json_group_array(json_object(...))` to aggregate child rows into a JSON array
162-
- Uses `COALESCE(..., '[]')` to handle empty results
160+
- `json_group_array(json_object(...))` inside a scalar subquery aggregates a row set into a JSON array of objects
161+
- The scalar subquery correlates against the outer row through its WHERE clause
162+
- `COALESCE(..., '[]')` yields an empty array when the row set is empty
163163

164164
**Example SQL Output:**
165165
```sql

test/integration/test/sql-orm-client/include.test.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -420,13 +420,13 @@ describe('integration/include', () => {
420420
timeouts.spinUpPpgDev,
421421
);
422422

423-
// TML-2595 worked example: the Pothos `totalCount` shape — a paginated
424-
// row branch alongside a count() scalar branch. This is the headline
425-
// case that motivated single-query combine emission: one correlated
423+
// Worked example: the Pothos `totalCount` shape — a paginated row
424+
// branch alongside a count() scalar branch. This is the headline case
425+
// that motivated single-query combine emission: one correlated
426426
// subquery packs both branches; the parent + count + page roll up to
427427
// one SQL execution per query.
428428
it(
429-
'TML-2595: include().combine({ recent: take(N), count: count() }) resolves in a single execution',
429+
'include().combine({ recent: take(N), count: count() }) resolves in a single execution',
430430
async () => {
431431
await withCollectionRuntime(async (runtime) => {
432432
const users = createUsersCollection(runtime);
@@ -928,9 +928,8 @@ describe('integration/include', () => {
928928
],
929929
},
930930
]);
931-
// TML-2594 acceptance: depth-2 on the default postgres test
932-
// contract must collapse to a single SQL execution via nested
933-
// correlated subqueries.
931+
// Depth-2 on the default postgres test contract must collapse to
932+
// a single SQL execution via nested correlated subqueries.
934933
expect(runtime.executions).toHaveLength(1);
935934
});
936935
},

test/integration/test/sql-orm-client/nested-includes-strategy.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ describe('integration/nested-includes/strategy', () => {
141141
const ast = execution?.ast;
142142
expect(isSelectAst(ast)).toBe(true);
143143
if (!isSelectAst(ast)) return;
144-
// No include join is emitted; the include rides on a
145-
// SubqueryExpr projection instead.
146-
expect((ast.joins ?? []).some((join) => join.lateral)).toBe(false);
144+
// No join at all is emitted (a fortiori no lateral join); the
145+
// include rides on a `SubqueryExpr` projection instead.
146+
expect(ast.joins ?? []).toEqual([]);
147147
expect(ast.projection.some((item) => item.alias === 'posts')).toBe(true);
148148
// The lowered SQL carries no LATERAL keyword either.
149149
expect(execution?.sql).not.toContain('LATERAL');

0 commit comments

Comments
 (0)