diff --git a/packages/3-extensions/sql-orm-client/src/include-strategy.ts b/packages/3-extensions/sql-orm-client/src/include-strategy.ts index 1c2a424011..acd7fc2e23 100644 --- a/packages/3-extensions/sql-orm-client/src/include-strategy.ts +++ b/packages/3-extensions/sql-orm-client/src/include-strategy.ts @@ -3,10 +3,29 @@ import type { SqlStorage } from '@prisma-next/sql-contract/types'; export type IncludeStrategy = 'lateral' | 'correlated' | 'multiQuery'; +/** + * Choose the SQL emission strategy for nested includes based on the + * contract's declared capabilities. + * + * - `'lateral'`: outer SELECT with one LATERAL JOIN per relation, + * aggregating to JSON. Requires both `lateral` and `jsonAgg`. + * Postgres has both. + * - `'correlated'`: outer SELECT with one correlated subquery per + * relation, aggregating to JSON. Requires `jsonAgg` only. + * SQLite has `jsonAgg` (via `json_group_array`) but no LATERAL. + * - `'multiQuery'`: fallback. One SELECT per relation, stitched + * together in JS via `WHERE pk IN (parent-pk-values)`. Always + * correct; just N+1 round-trips. + * + * The capability flags are looked up under the contract's + * `targetFamily` and `target` namespaces — the two layers the contract + * emitter actually populates. Cross-namespace ("`postgres.lateral` + * found while running SQLite") false positives are impossible because + * we only inspect the running target's namespaces. + */ export function selectIncludeStrategy(contract: Contract): IncludeStrategy { - const capabilities = contract.capabilities as Record | undefined; - const hasLateral = hasCapability(capabilities?.['lateral']); - const hasJsonAgg = hasCapability(capabilities?.['jsonAgg']); + const hasLateral = capabilityFlag(contract, 'lateral'); + const hasJsonAgg = capabilityFlag(contract, 'jsonAgg'); if (hasLateral && hasJsonAgg) { return 'lateral'; @@ -19,15 +38,18 @@ export function selectIncludeStrategy(contract: Contract): IncludeSt return 'multiQuery'; } -function hasCapability(value: unknown): boolean { - if (value === true) { - return true; - } - - if (typeof value !== 'object' || value === null) { - return false; - } - - const flags = value as Record; - return Object.values(flags).some((flag) => flag === true); +/** + * Read a capability flag from the contract's target/family namespaces. + * + * The contract emitter populates `capabilities[targetFamily]` (universal + * SQL flags like `jsonAgg`, `returning`) and `capabilities[target]` + * (target-specific flags like `lateral` on Postgres). Either may + * declare a given flag; the family namespace declares the floor and the + * target namespace can extend on top. + */ +function capabilityFlag(contract: Contract, flag: string): boolean { + return ( + contract.capabilities[contract.targetFamily]?.[flag] === true || + contract.capabilities[contract.target]?.[flag] === true + ); } diff --git a/packages/3-extensions/sql-orm-client/test/collection-dispatch.test.ts b/packages/3-extensions/sql-orm-client/test/collection-dispatch.test.ts index bdb06a1bed..fa2c796842 100644 --- a/packages/3-extensions/sql-orm-client/test/collection-dispatch.test.ts +++ b/packages/3-extensions/sql-orm-client/test/collection-dispatch.test.ts @@ -6,17 +6,35 @@ import type { IncludeExpr } from '../src/types'; import { emptyState } from '../src/types'; import { createCollectionFor } from './collection-fixtures'; import type { MockRuntime, TestContract } from './helpers'; -import { createMockRuntime, getTestContract } from './helpers'; - -function withSingleQueryCapabilities(contract: TestContract): TestContract { - return { - ...contract, - capabilities: { - ...contract.capabilities, - lateral: { enabled: true }, - jsonAgg: { enabled: true }, +import { createMockRuntime, getTestContract, withCapabilities } from './helpers'; + +function withSingleQueryCapabilities(contract: TestContract) { + return withCapabilities(contract, { + ...contract.capabilities, + [contract.targetFamily]: { + ...(contract.capabilities[contract.targetFamily] ?? {}), + jsonAgg: true, }, - } as unknown as TestContract; + [contract.target]: { + ...(contract.capabilities[contract.target] ?? {}), + jsonAgg: true, + lateral: true, + }, + }); +} + +/** + * Mirrors the shape produced by the contract emitter: capability flags + * nested under the family + target namespaces, with no top-level entries. + * Used to assert "single-query path is selected for an emitted-shape + * contract" — the regression scenario the principled namespaced lookup + * was introduced to handle. + */ +function withEmittedSqlCapabilities(contract: TestContract) { + return withCapabilities(contract, { + sql: { jsonAgg: true, returning: true }, + postgres: { jsonAgg: true, lateral: true, returning: true }, + }); } function addConnection( @@ -71,6 +89,36 @@ describe('collection-dispatch', () => { expect(rows).toEqual([{ id: 1, name: 'Alice', email: 'alice@example.com' }]); }); + it('dispatchCollectionRows() depth-1 include with emitted-shape capabilities fires a single SQL execution (regression guard for namespaced capability lookup)', async () => { + // Guards against regressing the fix that taught `selectIncludeStrategy` + // to read capability flags from the contract's `targetFamily` and + // `target` namespaces. Prior to that fix, every emitted contract fell + // back to multi-query for nested includes — silently, because + // functional correctness was unaffected. This test fails fast if the + // regression returns: an emitted-shape contract should resolve a + // depth-1 include in one SQL execution, not two. + const contract = withEmittedSqlCapabilities(getTestContract()); + const { collection, runtime } = createCollectionFor('User', contract); + const scoped = collection.select('name').include('posts'); + runtime.setNextResults([ + [{ id: 1, name: 'Alice', posts: '[{"id":10,"title":"Post A","user_id":1,"views":3}]' }], + ]); + + const rows = await dispatchCollectionRows>({ + contract, + runtime, + state: scoped.state, + tableName: scoped.tableName, + modelName: scoped.modelName, + }).toArray(); + + expect(rows).toEqual([ + { name: 'Alice', posts: [{ id: 10, title: 'Post A', userId: 1, views: 3 }] }, + ]); + // The point of the test: 1 execution, not N+1. + expect(runtime.executions).toHaveLength(1); + }); + it('dispatchCollectionRows() single-query path returns empty rows and releases scope', async () => { const contract = withSingleQueryCapabilities(getTestContract()); const { collection, runtime } = createCollectionFor('User', contract); @@ -195,7 +243,10 @@ describe('collection-dispatch', () => { }); it('dispatchCollectionRows() multi-query path stitches includes, strips hidden fields, and releases scope', async () => { - const contract = getTestContract(); + // Force multi-query strategy by clearing capabilities. Otherwise + // the base test contract's postgres.lateral / postgres.jsonAgg + // would route to single-query lateral. + const contract = withCapabilities(getTestContract(), {}); const { collection, runtime } = createCollectionFor('User', contract); const scoped = collection.select('name').include('posts', (posts) => posts.select('title')); @@ -237,13 +288,18 @@ describe('collection-dispatch', () => { }); it('dispatchCollectionRows() multi-query path handles empty parent result sets', async () => { - const { collection, runtime } = createCollectionFor('User'); + // Force multi-query strategy so the empty-parent early return inside + // `dispatchWithMultiQueryIncludes` is actually exercised. The base + // contract's postgres.lateral / postgres.jsonAgg would otherwise + // route to single-query lateral. + const contract = withCapabilities(getTestContract(), {}); + const { collection, runtime } = createCollectionFor('User', contract); const scoped = collection.include('posts'); runtime.setNextResults([[]]); const rows = await dispatchCollectionRows>({ - contract: collection.ctx.context.contract, + contract, runtime, state: scoped.state, tableName: scoped.tableName, diff --git a/packages/3-extensions/sql-orm-client/test/collection-fixtures.ts b/packages/3-extensions/sql-orm-client/test/collection-fixtures.ts index dbce9fb938..827006a923 100644 --- a/packages/3-extensions/sql-orm-client/test/collection-fixtures.ts +++ b/packages/3-extensions/sql-orm-client/test/collection-fixtures.ts @@ -1,3 +1,5 @@ +import type { Contract } from '@prisma-next/contract/types'; +import type { SqlStorage } from '@prisma-next/sql-contract/types'; import type { ExecutionContext } from '@prisma-next/sql-relational-core/query-lane-context'; import { Collection } from '../src/collection'; import type { MockRuntime, TestContract } from './helpers'; @@ -7,7 +9,7 @@ export type TestModelName = Extract; export const baseContract = getTestContract(); -function contextForContract(contract: TestContract): ExecutionContext { +function contextForContract(contract: Contract): ExecutionContext { const base = getTestContext(); if (contract === baseContract) return base; return { ...base, contract } as ExecutionContext; @@ -15,7 +17,7 @@ function contextForContract(contract: TestContract): ExecutionContext( modelName: ModelName, - contract: TestContract = baseContract, + contract: Contract = baseContract, ): { collection: Collection; runtime: MockRuntime; diff --git a/packages/3-extensions/sql-orm-client/test/helpers.ts b/packages/3-extensions/sql-orm-client/test/helpers.ts index aa1337ec9d..2e63fe280e 100644 --- a/packages/3-extensions/sql-orm-client/test/helpers.ts +++ b/packages/3-extensions/sql-orm-client/test/helpers.ts @@ -24,6 +24,26 @@ export function getTestContract(): TestContract { return structuredClone(baseTestContract); } +/** + * Override the capabilities of a {@link TestContract} for a test scenario. + * + * The narrow `TestContract` type fixes `capabilities` to the literal shape + * generated for `fixtures/generated/contract.json`. Tests need contracts + * with arbitrary capability shapes — empty, only-jsonAgg, cross-namespace, + * etc. — and want the override's literal types preserved so capability- + * dependent type checks remain meaningful. + * + * The result widens `TestContract`'s `capabilities` slot to the caller's + * `TCaps`, which the framework `Contract` interface already permits + * (`capabilities: Record>`). + */ +export function withCapabilities>>( + contract: TestContract, + capabilities: TCaps, +): Omit & { readonly capabilities: TCaps } { + return { ...contract, capabilities }; +} + const testContext: ExecutionContext = createExecutionContext({ contract: baseTestContract, stack: createSqlExecutionStack({ diff --git a/packages/3-extensions/sql-orm-client/test/include-strategy.test.ts b/packages/3-extensions/sql-orm-client/test/include-strategy.test.ts index fe43ebf275..15c74eabfd 100644 --- a/packages/3-extensions/sql-orm-client/test/include-strategy.test.ts +++ b/packages/3-extensions/sql-orm-client/test/include-strategy.test.ts @@ -1,82 +1,76 @@ import { describe, expect, it } from 'vitest'; import { selectIncludeStrategy } from '../src/include-strategy'; -import { getTestContract } from './helpers'; +import { getTestContract, withCapabilities } from './helpers'; + +// The default test contract has `target: 'postgres'`, `targetFamily: 'sql'`, +// and capabilities populated under those two namespaces. The strategy +// selector reads only those namespaces, so each test uses +// `withCapabilities(...)` to swap in the override the scenario needs. describe('selectIncludeStrategy', () => { it('returns multiQuery when include capabilities are absent', () => { - const contract = getTestContract(); - const strategy = selectIncludeStrategy(contract); + const contract = withCapabilities(getTestContract(), {}); + + expect(selectIncludeStrategy(contract)).toBe('multiQuery'); + }); + + it('returns correlated when jsonAgg is enabled in the family namespace without lateral', () => { + const contract = withCapabilities(getTestContract(), { + sql: { jsonAgg: true }, + }); - expect(strategy).toBe('multiQuery'); + expect(selectIncludeStrategy(contract)).toBe('correlated'); }); - it('returns correlated when jsonAgg is enabled without lateral', () => { - const contract = { - ...getTestContract(), - capabilities: { - jsonAgg: { - enabled: true, - }, - }, - }; + it('returns lateral when both flags are enabled in the same namespace', () => { + const contract = withCapabilities(getTestContract(), { + postgres: { jsonAgg: true, lateral: true }, + }); - const strategy = selectIncludeStrategy(contract); - expect(strategy).toBe('correlated'); + expect(selectIncludeStrategy(contract)).toBe('lateral'); }); - it('returns lateral when both lateral and jsonAgg are enabled', () => { - const contract = { - ...getTestContract(), - capabilities: { - lateral: { - enabled: true, - }, - jsonAgg: { - enabled: true, - }, - }, - }; + it('returns lateral when flags are split across family and target namespaces', () => { + // Real-world shape: SQL family declares `jsonAgg`; the postgres + // target adds `lateral` on top. + const contract = withCapabilities(getTestContract(), { + sql: { jsonAgg: true }, + postgres: { lateral: true }, + }); - const strategy = selectIncludeStrategy(contract); - expect(strategy).toBe('lateral'); + expect(selectIncludeStrategy(contract)).toBe('lateral'); }); - it('reads object capabilities via enabled flag', () => { - const contract = { - ...getTestContract(), - capabilities: { - lateral: { enabled: true }, - jsonAgg: { enabled: false, fallback: true }, - }, - }; + it('ignores capability flags in unrelated namespaces', () => { + // The default test contract's target/family are 'postgres' / 'sql'. + // A `mongo: { lateral: true }` namespace must not enable lateral + // on a postgres runtime — namespaces are scoped to the running + // target/family. + const contract = withCapabilities(getTestContract(), { + mongo: { jsonAgg: true, lateral: true }, + nonsense: { lateral: true }, + }); - const strategy = selectIncludeStrategy(contract); - expect(strategy).toBe('lateral'); + expect(selectIncludeStrategy(contract)).toBe('multiQuery'); }); - it('accepts top-level boolean capability flags', () => { - const contract = { - ...getTestContract(), - capabilities: { - lateral: true, - jsonAgg: true, - }, - } as unknown as ReturnType; + it('treats non-boolean capability values as missing', () => { + // The Contract type declares capability values as `boolean`. Anything + // else (string, object, undefined) is treated as not present. + // The cast on `'yes'` is deliberate — we're feeding an invalid value + // through a valid-typed contract to exercise the runtime check. + const contract = withCapabilities(getTestContract(), { + sql: { jsonAgg: 'yes' as unknown as boolean, lateral: true }, + }); - const strategy = selectIncludeStrategy(contract); - expect(strategy).toBe('lateral'); + expect(selectIncludeStrategy(contract)).toBe('multiQuery'); }); - it('ignores non-boolean, non-object capability values', () => { - const contract = { - ...getTestContract(), - capabilities: { - lateral: 'yes', - jsonAgg: true, - }, - } as unknown as ReturnType; + it('treats explicit `false` as not enabled', () => { + const contract = withCapabilities(getTestContract(), { + sql: { jsonAgg: true, lateral: false }, + }); - const strategy = selectIncludeStrategy(contract); - expect(strategy).toBe('correlated'); + expect(selectIncludeStrategy(contract)).toBe('correlated'); }); }); diff --git a/packages/3-extensions/sql-orm-client/test/integration/include.test.ts b/packages/3-extensions/sql-orm-client/test/integration/include.test.ts index 9c09504542..ff44583db2 100644 --- a/packages/3-extensions/sql-orm-client/test/integration/include.test.ts +++ b/packages/3-extensions/sql-orm-client/test/integration/include.test.ts @@ -47,12 +47,14 @@ function createUsersCollectionWithCapabilities( capabilities: Record, ) { const base = getTestContract(); + // Replace capabilities entirely (rather than merging with base) so the + // test's intent is unambiguous. Merging with the base contract's + // postgres namespace would leak `postgres.lateral` and `postgres.jsonAgg` + // into the strategy detection — making it impossible to test the + // correlated-only path by passing only `jsonAgg`. const contract = { ...base, - capabilities: { - ...base.capabilities, - ...capabilities, - }, + capabilities, } as typeof base; const context = { ...getTestContext(), contract }; @@ -65,6 +67,41 @@ type NumericPostField = import('../../src/types').NumericFieldNames< >; describe('integration/include', () => { + it( + 'depth-1 include against an emitted contract fires a single SQL execution (regression guard for namespaced capability lookup)', + async () => { + // Guards against regressing the fix that taught `selectIncludeStrategy` + // to read capability flags from the contract's `targetFamily` and + // `target` namespaces. The default `getTestContract()` carries + // `postgres: { lateral: true, jsonAgg: true, ... }` — the emitter's + // actual output shape. Prior to the fix, this exact test would fire + // 2 SQL queries instead of 1, against a real driver. + await withCollectionRuntime(async (runtime) => { + const users = createUsersCollection(runtime); + + await seedUsers(runtime, [{ id: 1, name: 'Alice', email: 'alice@example.com' }]); + await seedPosts(runtime, [{ id: 10, title: 'Post A', userId: 1, views: 100 }]); + + runtime.resetExecutions(); + const rows = await users.include('posts').all(); + + expect(rows).toEqual([ + { + id: 1, + name: 'Alice', + email: 'alice@example.com', + invitedById: null, + address: null, + posts: [{ id: 10, title: 'Post A', userId: 1, views: 100, embedding: null }], + }, + ]); + // The point of the test: 1 execution, not N+1. + expect(runtime.executions).toHaveLength(1); + }); + }, + timeouts.spinUpPpgDev, + ); + it( 'include() stitches one-to-many and one-to-one relations from real rows', async () => { @@ -333,8 +370,7 @@ describe('integration/include', () => { async () => { await withCollectionRuntime(async (runtime) => { const users = createUsersCollectionWithCapabilities(runtime, { - lateral: { enabled: true }, - jsonAgg: { enabled: true }, + postgres: { jsonAgg: true, lateral: true }, }); await seedUsers(runtime, [ @@ -412,8 +448,7 @@ describe('integration/include', () => { async () => { await withCollectionRuntime(async (runtime) => { const users = createUsersCollectionWithCapabilities(runtime, { - lateral: { enabled: true }, - jsonAgg: { enabled: true }, + postgres: { jsonAgg: true, lateral: true }, }); await seedUsers(runtime, [ @@ -506,7 +541,8 @@ describe('integration/include', () => { async () => { await withCollectionRuntime(async (runtime) => { const users = createUsersCollectionWithCapabilities(runtime, { - jsonAgg: { enabled: true }, + // jsonAgg without lateral → correlated subquery strategy. + sql: { jsonAgg: true }, }); await seedUsers(runtime, [{ id: 1, name: 'Alice', email: 'alice@example.com' }]); @@ -547,7 +583,8 @@ describe('integration/include', () => { async () => { await withCollectionRuntime(async (runtime) => { const users = createUsersCollectionWithCapabilities(runtime, { - jsonAgg: { enabled: true }, + // jsonAgg without lateral → correlated subquery strategy. + sql: { jsonAgg: true }, }); await seedUsers(runtime, [