diff --git a/docs/releases/v8.0.0-rc.5.md b/docs/releases/v8.0.0-rc.5.md index 92e6f20cc9e5..ff567a497c97 100644 --- a/docs/releases/v8.0.0-rc.5.md +++ b/docs/releases/v8.0.0-rc.5.md @@ -30,6 +30,7 @@ The upgrade recipe for this hop: the [user recipe](https://github.com/prisma/pri - `aggregate()` now reduces over exactly the rows a chain's `take` / `skip` / `cursor` / `distinct` / `distinctOn` describes, instead of silently reducing over every matching row. ([#30067](https://github.com/prisma/prisma/pull/30067)) - `groupBy()` now scopes pre-group pagination to the rows it groups instead of dropping it, and `GroupedCollection` gained `take` / `skip` / `orderBy` to page the groups themselves. ([#30092](https://github.com/prisma/prisma/pull/30092)) +- `ORDER BY` / `DISTINCT ON` on an enum column now sort by declaration order behind a derived table (`distinct()`, `groupBy()`), instead of silently falling back to lexical text order. ([#30099](https://github.com/prisma/prisma/pull/30099)) - The Postgres runtime attaches `'error'` listeners to every pool and client it creates or receives, so a dropped idle connection (database restart, pooler timeout, network blip) no longer crashes the process as an uncaught exception. Pools your own code constructs and uses directly still need a listener — see the [upgrade recipe](https://github.com/prisma/prisma/tree/v8.0.0-rc.5/skills/prisma-next-upgrade/upgrades/8.0.0-rc.4-to-8.0.0-rc.5/). ([#30081](https://github.com/prisma/prisma/pull/30081)) - The PSL language server recognizes connection errors raised by any bundled copy of vscode-jsonrpc, instead of crashing when a duplicated copy raised them. ([#30077](https://github.com/prisma/prisma/pull/30077)) - CLI error text interpolates the configured migrations directory instead of assuming the default path. ([#30041](https://github.com/prisma/prisma/pull/30041)) diff --git a/packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts b/packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts index 24579d30b616..106c56ce8443 100644 --- a/packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts +++ b/packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts @@ -201,8 +201,7 @@ function renderLimitOffset( } function renderSelect(ast: SelectAst, contract: PostgresContract, pim: ParamIndexMap): string { - const sourcesByRef = collectTableSources(ast); - const selectClause = `SELECT ${renderDistinctPrefix(ast.distinct, ast.distinctOn, sourcesByRef, contract, pim)}${renderProjection( + const selectClause = `SELECT ${renderDistinctPrefix(ast.distinct, ast.distinctOn, ast, contract, pim)}${renderProjection( ast.projection, contract, pim, @@ -221,7 +220,7 @@ function renderSelect(ast: SelectAst, contract: PostgresContract, pim: ParamInde const orderClause = ast.orderBy?.length ? `ORDER BY ${ast.orderBy .map((order) => { - const expr = renderOrderByExpr(order.expr, sourcesByRef, contract, pim); + const expr = renderOrderByExpr(order.expr, ast, contract, pim); return `${expr} ${order.dir.toUpperCase()}`; }) .join(', ')}` @@ -246,59 +245,103 @@ function renderSelect(ast: SelectAst, contract: PostgresContract, pim: ParamInde } /** - * Storage coordinate a query-level table reference (alias or bare name) resolves to. The ORDER BY enum hook uses this to look a column-ref's storage column up and read its value-set. + * The FROM/JOIN source a query-level table reference (alias or bare name) + * resolves to. The ORDER BY enum hook uses this to look a column-ref's + * storage column up and read its value-set. */ -interface TableSourceCoordinate { - readonly name: string; - readonly namespaceId: string | undefined; +function findFromSource(ast: SelectAst, ref: string): AnyFromSource | undefined { + const refOf = (source: AnyFromSource): string | undefined => + source.kind === 'table-source' ? (source.alias ?? source.name) : source.alias; + if (ast.from !== undefined && refOf(ast.from) === ref) { + return ast.from; + } + for (const join of ast.joins ?? []) { + if (refOf(join.source) === ref) { + return join.source; + } + } + return undefined; } /** - * Map a SELECT's table references (the FROM source and any JOIN sources) to their storage coordinate, keyed by the name a `ColumnRef.table` would carry (the alias when present, otherwise the table name). Derived-table sources are skipped — their columns are projected through a sub-select, not a base storage column, so the enum hook does not apply. + * Whether a FROM/JOIN source carries a column of the given name, and the + * ordered value-set it resolves to (`undefined` when the column carries no + * value-set — the common, non-enum case). `found: false` distinguishes "no + * such column" from "column exists, not enum-backed", which the unqualified + * identifier resolver needs for its ambiguity check. + * + * A `table-source` resolves the column directly against the contract's + * storage. A `derived-table-source` has no storage column of its own — its + * columns are projected through a sub-select — so this looks up the + * matching `ProjectionItem` by output alias and, when the projected + * expression is itself a plain `ColumnRef`, recurses into the derived + * table's own query to resolve *that* reference, so nested wraps resolve + * too. A projected expression that isn't a plain column reference (a + * function call, a literal, …) has no storage column to trace back to; + * this reports `found: true` with no value-set, which correctly falls + * through to plain-column rendering rather than guessing at an order. */ -function collectTableSources(ast: SelectAst): ReadonlyMap { - const sources = new Map(); - const add = (source: AnyFromSource): void => { - if (source.kind !== 'table-source') { - return; +function resolveColumnValueSetFromSource( + source: AnyFromSource, + column: string, + contract: PostgresContract, +): { readonly found: boolean; readonly values: readonly JsonValue[] | undefined } { + if (source.kind === 'table-source') { + if (source.namespaceId === undefined) { + return { found: false, values: undefined }; } - const ref = source.alias ?? source.name; - sources.set(ref, { name: source.name, namespaceId: source.namespaceId }); - }; - if (ast.from !== undefined) add(ast.from); - for (const join of ast.joins ?? []) { - add(join.source); + const ns = contract.storage.namespaces[source.namespaceId]; + const storageColumn = ns?.entries.table?.[source.name]?.columns[column]; + if (storageColumn === undefined) { + return { found: false, values: undefined }; + } + const valueSet = storageColumn.valueSet; + if (valueSet === undefined) { + return { found: true, values: undefined }; + } + const valueSetNs = contract.storage.namespaces[valueSet.namespaceId]; + return { found: true, values: valueSetNs?.entries.valueSet?.[valueSet.entityName]?.values }; + } + if (source.kind === 'derived-table-source') { + const projected = source.query.projection.find((item) => item.alias === column); + if (projected === undefined) { + return { found: false, values: undefined }; + } + if (projected.expr.kind !== 'column-ref') { + return { found: true, values: undefined }; + } + return resolveColumnValueSet(source.query, projected.expr, contract); } - return sources; + return { found: false, values: undefined }; } /** - * Ordered, codec-encoded values of the value-set a storage column restricts to, or `undefined` when the referenced column carries no value-set (the common, non-enum case). Resolves the column's storage coordinate from the SELECT's table sources, then the column's `valueSet` ref to the value-set's `values`. + * Ordered, codec-encoded values of the value-set a storage column restricts + * to, or `undefined` when the reference doesn't resolve to one (no matching + * source, no such column, or a column that isn't enum-backed). */ function allStrings(values: readonly JsonValue[]): values is readonly string[] { return values.every((value) => typeof value === 'string'); } +function resolveColumnValueSet( + ast: SelectAst, + ref: ColumnRef, + contract: PostgresContract, +): { readonly found: boolean; readonly values: readonly JsonValue[] | undefined } { + const source = findFromSource(ast, ref.table); + if (source === undefined) { + return { found: false, values: undefined }; + } + return resolveColumnValueSetFromSource(source, ref.column, contract); +} + function resolveEnumOrderValues( ref: ColumnRef, - sourcesByRef: ReadonlyMap, + ast: SelectAst, contract: PostgresContract, ): readonly JsonValue[] | undefined { - const source = sourcesByRef.get(ref.table); - if (source === undefined || source.namespaceId === undefined) { - return undefined; - } - const sourceNs = contract.storage.namespaces[source.namespaceId]; - const column = - sourceNs !== undefined ? sourceNs.entries.table?.[source.name]?.columns[ref.column] : undefined; - const valueSet = column?.valueSet; - if (valueSet === undefined) { - return undefined; - } - const valueSetNs = contract.storage.namespaces[valueSet.namespaceId]; - return valueSetNs !== undefined - ? valueSetNs.entries.valueSet?.[valueSet.entityName]?.values - : undefined; + return resolveColumnValueSet(ast, ref, contract).values; } /** @@ -306,34 +349,24 @@ function resolveEnumOrderValues( */ function resolveEnumOrderValuesForIdentifier( name: string, - sourcesByRef: ReadonlyMap, + ast: SelectAst, contract: PostgresContract, ): readonly JsonValue[] | undefined { + const candidates = [ast.from, ...(ast.joins ?? []).map((join) => join.source)].filter( + (source): source is AnyFromSource => source !== undefined, + ); let matchedColumns = 0; let resolved: readonly JsonValue[] | undefined; - for (const source of sourcesByRef.values()) { - if (source.namespaceId === undefined) { - continue; - } - const identNs = contract.storage.namespaces[source.namespaceId]; - const column = - identNs !== undefined ? identNs.entries.table?.[source.name]?.columns[name] : undefined; - if (column === undefined) { + for (const source of candidates) { + const { found, values } = resolveColumnValueSetFromSource(source, name, contract); + if (!found) { continue; } matchedColumns += 1; if (matchedColumns > 1) { return undefined; } - const valueSet = column.valueSet; - if (valueSet === undefined) { - return undefined; - } - const valueSetNs = contract.storage.namespaces[valueSet.namespaceId]; - resolved = - valueSetNs !== undefined - ? valueSetNs.entries.valueSet?.[valueSet.entityName]?.values - : undefined; + resolved = values; } return resolved; } @@ -343,7 +376,7 @@ function resolveEnumOrderValuesForIdentifier( */ function renderOrderByExpr( expr: AnyExpression, - sourcesByRef: ReadonlyMap, + ast: SelectAst, contract: PostgresContract, pim: ParamIndexMap, ): string { @@ -352,14 +385,14 @@ function renderOrderByExpr( // value-set falls through to plain column rendering rather than emitting a // wrong numeric-as-text ARRAY. if (expr.kind === 'column-ref') { - const orderValues = resolveEnumOrderValues(expr, sourcesByRef, contract); + const orderValues = resolveEnumOrderValues(expr, ast, contract); if (orderValues !== undefined && allStrings(orderValues)) { const array = orderValues.map((value) => `'${escapeLiteral(value)}'`).join(', '); return `array_position(ARRAY[${array}]::text[], ${renderColumn(expr)})`; } } if (expr.kind === 'identifier-ref') { - const orderValues = resolveEnumOrderValuesForIdentifier(expr.name, sourcesByRef, contract); + const orderValues = resolveEnumOrderValuesForIdentifier(expr.name, ast, contract); if (orderValues !== undefined && allStrings(orderValues)) { const array = orderValues.map((value) => `'${escapeLiteral(value)}'`).join(', '); return `array_position(ARRAY[${array}]::text[], ${quoteIdentifier(expr.name)})`; @@ -408,13 +441,13 @@ function renderReturning( function renderDistinctPrefix( distinct: true | undefined, distinctOn: ReadonlyArray | undefined, - sourcesByRef: ReadonlyMap, + ast: SelectAst, contract: PostgresContract, pim: ParamIndexMap, ): string { if (distinctOn && distinctOn.length > 0) { const rendered = distinctOn - .map((expr) => renderOrderByExpr(expr, sourcesByRef, contract, pim)) + .map((expr) => renderOrderByExpr(expr, ast, contract, pim)) .join(', '); return `DISTINCT ON (${rendered}) `; } diff --git a/packages/3-targets/6-adapters/postgres/test/migrations/order-by-enum.integration.test.ts b/packages/3-targets/6-adapters/postgres/test/migrations/order-by-enum.integration.test.ts index 9bdd84eee7b1..ecabcd2305b7 100644 --- a/packages/3-targets/6-adapters/postgres/test/migrations/order-by-enum.integration.test.ts +++ b/packages/3-targets/6-adapters/postgres/test/migrations/order-by-enum.integration.test.ts @@ -5,7 +5,9 @@ import { APP_SPACE_ID } from '@internal/framework-components/control'; import type { SqlStorage } from '@internal/sql-contract/types'; import { buildBoundContract, enumType, member } from '@internal/sql-contract-ts/contract-builder'; import { + AggregateExpr, ColumnRef, + DerivedTableSource, EqColJoinOn, IdentifierRef, JoinAst, @@ -318,3 +320,120 @@ describe('ORDER BY on an enum column — declaration order, PGlite', { concurren expect(lowered.sql).not.toContain('array_position'); }); }); + +// `collectTableSources` (sql-renderer.ts) only recognises `table-source` FROM +// entries, so the enum hook resolves nothing once the FROM is a derived +// table — even though the ORM aliases that derived table back to the base +// table name specifically so outer references keep resolving. This is not +// specific to a grouped aggregate: any derived-table wrap loses declaration +// order the same way, wherever it comes from — `distinct()`'s ROW_NUMBER +// dedup wrap on the plain-select path (case 1/1b), a grouped aggregate's +// pre-group scoping wrap (case 2), or DISTINCT ON sharing renderOrderByExpr +// with ORDER BY (case 3). Case 0 is the unwrapped control: it already works +// and must keep working once the wrap-aware fix lands. +describe('ORDER BY on an enum column behind a derived table', () => { + const contract = makeTaskContract(); + + it('case 0 (control): unwrapped ORDER BY on the enum column uses array_position', () => { + const ast = SelectAst.from(TableSource.named('Task', undefined, 'public')) + .withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]) + .withOrderBy([OrderByItem.asc(ColumnRef.of('Task', 'priority'))]); + + const lowered = createPostgresAdapter().lower(ast, { contract }); + expect(lowered.sql).toContain( + `array_position(ARRAY['low', 'high', 'medium']::text[], "Task"."priority")`, + ); + }); + + it('case 1: derived-table wrap, column-ref ORDER BY on the enum column', () => { + const inner = SelectAst.from(TableSource.named('Task', undefined, 'public')).withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]); + const ast = SelectAst.from(DerivedTableSource.as('Task', inner)) + .withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]) + .withOrderBy([OrderByItem.asc(ColumnRef.of('Task', 'priority'))]); + + const lowered = createPostgresAdapter().lower(ast, { contract }); + expect(lowered.sql).toContain( + `array_position(ARRAY['low', 'high', 'medium']::text[], "Task"."priority")`, + ); + }); + + it('case 1b: derived-table wrap, identifier-ref ORDER BY on the enum column', () => { + const inner = SelectAst.from(TableSource.named('Task', undefined, 'public')).withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]); + const ast = SelectAst.from(DerivedTableSource.as('Task', inner)) + .withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]) + .withOrderBy([OrderByItem.asc(IdentifierRef.of('priority'))]); + + const lowered = createPostgresAdapter().lower(ast, { contract }); + expect(lowered.sql).toContain( + `array_position(ARRAY['low', 'high', 'medium']::text[], "priority")`, + ); + }); + + it('case 2: derived-table wrap + GROUP BY + post-group ORDER BY on the group key', () => { + const inner = SelectAst.from(TableSource.named('Task', undefined, 'public')).withProjection([ + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]); + const ast = SelectAst.from(DerivedTableSource.as('Task', inner)) + .withProjection([ + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ProjectionItem.of('total', AggregateExpr.count()), + ]) + .withGroupBy([ColumnRef.of('Task', 'priority')]) + .withOrderBy([OrderByItem.asc(ColumnRef.of('Task', 'priority'))]) + .withLimit(1); + + const lowered = createPostgresAdapter().lower(ast, { contract }); + expect(lowered.sql).toContain( + `array_position(ARRAY['low', 'high', 'medium']::text[], "Task"."priority")`, + ); + }); + + it('case 3: derived-table wrap, DISTINCT ON the enum column', () => { + const inner = SelectAst.from(TableSource.named('Task', undefined, 'public')).withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority', ColumnRef.of('Task', 'priority')), + ]); + const ast = SelectAst.from(DerivedTableSource.as('Task', inner)) + .withProjection([ProjectionItem.of('id', ColumnRef.of('Task', 'id'))]) + .withDistinctOn([IdentifierRef.of('priority')]) + .withOrderBy([OrderByItem.asc(IdentifierRef.of('priority'))]); + + const lowered = createPostgresAdapter().lower(ast, { contract }); + const arrayPositionExpr = `array_position(ARRAY['low', 'high', 'medium']::text[], "priority")`; + expect(lowered.sql).toContain(`DISTINCT ON (${arrayPositionExpr})`); + expect(lowered.sql).toContain(`ORDER BY ${arrayPositionExpr}`); + }); + + // A projected expression that isn't a plain column reference (an alias + // over a function call, say) has no storage column to look up — falling + // back to a bare sort here is the status quo, not a new gap, and a wrong + // enum order would be worse than the bug this file exists to close. + it('falls back to plain column rendering when the derived table cannot resolve a storage column', () => { + const inner = SelectAst.from(TableSource.named('Task', undefined, 'public')).withProjection([ + ProjectionItem.of('id', ColumnRef.of('Task', 'id')), + ProjectionItem.of('priority_label', IdentifierRef.of('priority')), + ]); + const ast = SelectAst.from(DerivedTableSource.as('Task', inner)) + .withProjection([ProjectionItem.of('id', ColumnRef.of('Task', 'id'))]) + .withOrderBy([OrderByItem.asc(ColumnRef.of('Task', 'priority_label'))]); + + const lowered = createPostgresAdapter().lower(ast, { contract }); + expect(lowered.sql).not.toContain('array_position'); + expect(lowered.sql).toContain('ORDER BY "Task"."priority_label" ASC'); + }); +});