From 9c88af4b204515593e9885bff538b515ae4cf956 Mon Sep 17 00:00:00 2001 From: Alessio Gravili Date: Mon, 31 Mar 2025 14:37:45 -0600 Subject: [PATCH] refactor(drizzle): replace query chaining with dynamic query building (#11923) This replaces usage of our `chainMethods` helper to dynamically chain queries with [drizzle dynamic query building](https://orm.drizzle.team/docs/dynamic-query-building). This is more type-safe, more readable and requires less code --- packages/db-sqlite/src/countDistinct.ts | 30 ++++----- packages/drizzle/src/deleteOne.ts | 4 +- packages/drizzle/src/find/chainMethods.ts | 5 ++ packages/drizzle/src/find/findMany.ts | 21 ++---- packages/drizzle/src/find/traverseFields.ts | 64 ++++++++----------- .../drizzle/src/postgres/countDistinct.ts | 32 ++++------ .../drizzle/src/queries/selectDistinct.ts | 38 +++++------ packages/drizzle/src/types.ts | 8 ++- packages/drizzle/src/updateMany.ts | 32 ++-------- packages/drizzle/src/updateOne.ts | 2 +- 10 files changed, 93 insertions(+), 143 deletions(-) diff --git a/packages/db-sqlite/src/countDistinct.ts b/packages/db-sqlite/src/countDistinct.ts index 5a0ada7a9d..de0af7a995 100644 --- a/packages/db-sqlite/src/countDistinct.ts +++ b/packages/db-sqlite/src/countDistinct.ts @@ -1,6 +1,5 @@ -import type { ChainedMethods } from '@payloadcms/drizzle/types' +import type { SQLiteSelect } from 'drizzle-orm/sqlite-core' -import { chainMethods } from '@payloadcms/drizzle' import { count, sql } from 'drizzle-orm' import type { CountDistinct, SQLiteAdapter } from './types.js' @@ -20,30 +19,25 @@ export const countDistinct: CountDistinct = async function countDistinct( return Number(countResult[0]?.count) } - const chainedMethods: ChainedMethods = [] + let query: SQLiteSelect = db + .select({ + count: sql`COUNT(1) OVER()`, + }) + .from(this.tables[tableName]) + .where(where) + .groupBy(this.tables[tableName].id) + .limit(1) + .$dynamic() joins.forEach(({ condition, table }) => { - chainedMethods.push({ - args: [table, condition], - method: 'leftJoin', - }) + query = query.leftJoin(table, condition) }) // When we have any joins, we need to count each individual ID only once. // COUNT(*) doesn't work for this well in this case, as it also counts joined tables. // SELECT (COUNT DISTINCT id) has a very slow performance on large tables. // Instead, COUNT (GROUP BY id) can be used which is still slower than COUNT(*) but acceptable. - const countResult = await chainMethods({ - methods: chainedMethods, - query: db - .select({ - count: sql`COUNT(1) OVER()`, - }) - .from(this.tables[tableName]) - .where(where) - .groupBy(this.tables[tableName].id) - .limit(1), - }) + const countResult = await query return Number(countResult[0]?.count) } diff --git a/packages/drizzle/src/deleteOne.ts b/packages/drizzle/src/deleteOne.ts index 2c2d12c65b..760d78dec5 100644 --- a/packages/drizzle/src/deleteOne.ts +++ b/packages/drizzle/src/deleteOne.ts @@ -13,7 +13,7 @@ import { getTransaction } from './utilities/getTransaction.js' export const deleteOne: DeleteOne = async function deleteOne( this: DrizzleAdapter, - { collection: collectionSlug, req, select, where: whereArg, returning }, + { collection: collectionSlug, req, returning, select, where: whereArg }, ) { const db = await getTransaction(this, req) const collection = this.payload.collections[collectionSlug].config @@ -32,9 +32,9 @@ export const deleteOne: DeleteOne = async function deleteOne( const selectDistinctResult = await selectDistinct({ adapter: this, - chainedMethods: [{ args: [1], method: 'limit' }], db, joins, + query: ({ query }) => query.limit(1), selectFields, tableName, where, diff --git a/packages/drizzle/src/find/chainMethods.ts b/packages/drizzle/src/find/chainMethods.ts index 146ce641ad..3d8d59e76d 100644 --- a/packages/drizzle/src/find/chainMethods.ts +++ b/packages/drizzle/src/find/chainMethods.ts @@ -1,3 +1,6 @@ +/** + * @deprecated - will be removed in 4.0. Use query + $dynamic() instead: https://orm.drizzle.team/docs/dynamic-query-building + */ export type ChainedMethods = { args: unknown[] method: string @@ -7,6 +10,8 @@ export type ChainedMethods = { * Call and returning methods that would normally be chained together but cannot be because of control logic * @param methods * @param query + * + * @deprecated - will be removed in 4.0. Use query + $dynamic() instead: https://orm.drizzle.team/docs/dynamic-query-building */ const chainMethods = ({ methods, query }: { methods: ChainedMethods; query: T }): T => { return methods.reduce((query, { args, method }) => { diff --git a/packages/drizzle/src/find/findMany.ts b/packages/drizzle/src/find/findMany.ts index 6874e74403..d2ba6e129c 100644 --- a/packages/drizzle/src/find/findMany.ts +++ b/packages/drizzle/src/find/findMany.ts @@ -3,7 +3,6 @@ import type { FindArgs, FlattenedField, TypeWithID } from 'payload' import { inArray } from 'drizzle-orm' import type { DrizzleAdapter } from '../types.js' -import type { ChainedMethods } from './chainMethods.js' import buildQuery from '../queries/buildQuery.js' import { selectDistinct } from '../queries/selectDistinct.js' @@ -62,15 +61,6 @@ export const findMany = async function find({ const orderedIDMap: Record = {} let orderedIDs: (number | string)[] - const selectDistinctMethods: ChainedMethods = [] - - if (orderBy) { - selectDistinctMethods.push({ - args: [() => orderBy.map(({ column, order }) => order(column))], - method: 'orderBy', - }) - } - const findManyArgs = buildFindManyArgs({ adapter, collectionSlug, @@ -84,15 +74,16 @@ export const findMany = async function find({ tableName, versions, }) - - selectDistinctMethods.push({ args: [offset], method: 'offset' }) - selectDistinctMethods.push({ args: [limit], method: 'limit' }) - const selectDistinctResult = await selectDistinct({ adapter, - chainedMethods: selectDistinctMethods, db, joins, + query: ({ query }) => { + if (orderBy) { + query = query.orderBy(() => orderBy.map(({ column, order }) => order(column))) + } + return query.offset(offset).limit(limit) + }, selectFields, tableName, where, diff --git a/packages/drizzle/src/find/traverseFields.ts b/packages/drizzle/src/find/traverseFields.ts index 771033cc29..4bb2aae2fd 100644 --- a/packages/drizzle/src/find/traverseFields.ts +++ b/packages/drizzle/src/find/traverseFields.ts @@ -1,5 +1,5 @@ import type { LibSQLDatabase } from 'drizzle-orm/libsql' -import type { SQLiteSelectBase } from 'drizzle-orm/sqlite-core' +import type { SQLiteSelect, SQLiteSelectBase } from 'drizzle-orm/sqlite-core' import { and, asc, count, desc, eq, or, sql } from 'drizzle-orm' import { @@ -16,7 +16,7 @@ import { import { fieldIsVirtual, fieldShouldBeLocalized } from 'payload/shared' import toSnakeCase from 'to-snake-case' -import type { BuildQueryJoinAliases, ChainedMethods, DrizzleAdapter } from '../types.js' +import type { BuildQueryJoinAliases, DrizzleAdapter } from '../types.js' import type { Result } from './buildFindManyArgs.js' import buildQuery from '../queries/buildQuery.js' @@ -25,7 +25,6 @@ import { operatorMap } from '../queries/operatorMap.js' import { getNameFromDrizzleTable } from '../utilities/getNameFromDrizzleTable.js' import { jsonAggBuildObject } from '../utilities/json.js' import { rawConstraint } from '../utilities/rawConstraint.js' -import { chainMethods } from './chainMethods.js' const flattenAllWherePaths = (where: Where, paths: string[]) => { for (const k in where) { @@ -612,34 +611,6 @@ export const traverseFields = ({ where: joinQueryWhere, }) - const chainedMethods: ChainedMethods = [] - - joins.forEach(({ type, condition, table }) => { - chainedMethods.push({ - args: [table, condition], - method: type ?? 'leftJoin', - }) - }) - - if (page && limit !== 0) { - const offset = (page - 1) * limit - 1 - if (offset > 0) { - chainedMethods.push({ - args: [offset], - method: 'offset', - }) - } - } - - if (limit !== 0) { - chainedMethods.push({ - args: [limit], - method: 'limit', - }) - } - - const db = adapter.drizzle as LibSQLDatabase - for (let key in selectFields) { const val = selectFields[key] @@ -654,14 +625,29 @@ export const traverseFields = ({ selectFields.parent = newAliasTable.parent } - const subQuery = chainMethods({ - methods: chainedMethods, - query: db - .select(selectFields as any) - .from(newAliasTable) - .where(subQueryWhere) - .orderBy(() => orderBy.map(({ column, order }) => order(column))), - }).as(subQueryAlias) + let query: SQLiteSelect = db + .select(selectFields as any) + .from(newAliasTable) + .where(subQueryWhere) + .orderBy(() => orderBy.map(({ column, order }) => order(column))) + .$dynamic() + + joins.forEach(({ type, condition, table }) => { + query = query[type ?? 'leftJoin'](table, condition) + }) + + if (page && limit !== 0) { + const offset = (page - 1) * limit - 1 + if (offset > 0) { + query = query.offset(offset) + } + } + + if (limit !== 0) { + query = query.limit(limit) + } + + const subQuery = query.as(subQueryAlias) if (shouldCount) { currentArgs.extras[`${columnName}_count`] = sql`${db diff --git a/packages/drizzle/src/postgres/countDistinct.ts b/packages/drizzle/src/postgres/countDistinct.ts index 2f8f113ac4..16c2f576c9 100644 --- a/packages/drizzle/src/postgres/countDistinct.ts +++ b/packages/drizzle/src/postgres/countDistinct.ts @@ -1,10 +1,9 @@ +import type { PgTableWithColumns } from 'drizzle-orm/pg-core' + import { count, sql } from 'drizzle-orm' -import type { ChainedMethods } from '../types.js' import type { BasePostgresAdapter, CountDistinct } from './types.js' -import { chainMethods } from '../find/chainMethods.js' - export const countDistinct: CountDistinct = async function countDistinct( this: BasePostgresAdapter, { db, joins, tableName, where }, @@ -20,30 +19,25 @@ export const countDistinct: CountDistinct = async function countDistinct( return Number(countResult[0].count) } - const chainedMethods: ChainedMethods = [] + let query = db + .select({ + count: sql`COUNT(1) OVER()`, + }) + .from(this.tables[tableName]) + .where(where) + .groupBy(this.tables[tableName].id) + .limit(1) + .$dynamic() joins.forEach(({ condition, table }) => { - chainedMethods.push({ - args: [table, condition], - method: 'leftJoin', - }) + query = query.leftJoin(table as PgTableWithColumns, condition) }) // When we have any joins, we need to count each individual ID only once. // COUNT(*) doesn't work for this well in this case, as it also counts joined tables. // SELECT (COUNT DISTINCT id) has a very slow performance on large tables. // Instead, COUNT (GROUP BY id) can be used which is still slower than COUNT(*) but acceptable. - const countResult = await chainMethods({ - methods: chainedMethods, - query: db - .select({ - count: sql`COUNT(1) OVER()`, - }) - .from(this.tables[tableName]) - .where(where) - .groupBy(this.tables[tableName].id) - .limit(1), - }) + const countResult = await query return Number(countResult[0].count) } diff --git a/packages/drizzle/src/queries/selectDistinct.ts b/packages/drizzle/src/queries/selectDistinct.ts index 3e393dd82b..6354992f83 100644 --- a/packages/drizzle/src/queries/selectDistinct.ts +++ b/packages/drizzle/src/queries/selectDistinct.ts @@ -1,7 +1,7 @@ import type { QueryPromise, SQL } from 'drizzle-orm' -import type { SQLiteColumn } from 'drizzle-orm/sqlite-core' +import type { PgSelect } from 'drizzle-orm/pg-core' +import type { SQLiteColumn, SQLiteSelect } from 'drizzle-orm/sqlite-core' -import type { ChainedMethods } from '../find/chainMethods.js' import type { DrizzleAdapter, DrizzleTransaction, @@ -12,13 +12,11 @@ import type { } from '../types.js' import type { BuildQueryJoinAliases } from './buildQuery.js' -import { chainMethods } from '../find/chainMethods.js' - type Args = { adapter: DrizzleAdapter - chainedMethods?: ChainedMethods db: DrizzleAdapter['drizzle'] | DrizzleTransaction joins: BuildQueryJoinAliases + query?: (args: { query: SQLiteSelect }) => SQLiteSelect selectFields: Record tableName: string where: SQL @@ -29,42 +27,40 @@ type Args = { */ export const selectDistinct = ({ adapter, - chainedMethods = [], db, joins, + query: queryModifier = ({ query }) => query, selectFields, tableName, where, }: Args): QueryPromise<{ id: number | string }[] & Record> => { if (Object.keys(joins).length > 0) { - if (where) { - chainedMethods.push({ args: [where], method: 'where' }) - } - - joins.forEach(({ condition, table }) => { - chainedMethods.push({ - args: [table, condition], - method: 'leftJoin', - }) - }) - - let query + let query: SQLiteSelect const table = adapter.tables[tableName] if (adapter.name === 'postgres') { query = (db as TransactionPg) .selectDistinct(selectFields as Record) .from(table) + .$dynamic() as unknown as SQLiteSelect } if (adapter.name === 'sqlite') { query = (db as TransactionSQLite) .selectDistinct(selectFields as Record) .from(table) + .$dynamic() } - return chainMethods({ - methods: chainedMethods, - query, + if (where) { + query = query.where(where) + } + + joins.forEach(({ condition, table }) => { + query = query.leftJoin(table, condition) }) + + return queryModifier({ + query, + }) as unknown as QueryPromise<{ id: number | string }[] & Record> } } diff --git a/packages/drizzle/src/types.ts b/packages/drizzle/src/types.ts index da6f148f1f..b34ab6384d 100644 --- a/packages/drizzle/src/types.ts +++ b/packages/drizzle/src/types.ts @@ -37,11 +37,8 @@ import type { DrizzleSnapshotJSON } from 'drizzle-kit/api' import type { SQLiteRaw } from 'drizzle-orm/sqlite-core/query-builders/raw' import type { QueryResult } from 'pg' -import type { ChainedMethods } from './find/chainMethods.js' import type { Operators } from './queries/operatorMap.js' -export { ChainedMethods } - export type PostgresDB = NodePgDatabase> export type SQLiteDB = LibSQLDatabase< @@ -377,3 +374,8 @@ export type RelationMap = Map< type: 'many' | 'one' } > + +/** + * @deprecated - will be removed in 4.0. Use query + $dynamic() instead: https://orm.drizzle.team/docs/dynamic-query-building + */ +export type { ChainedMethods } from './find/chainMethods.js' diff --git a/packages/drizzle/src/updateMany.ts b/packages/drizzle/src/updateMany.ts index 8cde607a59..9cd979fe42 100644 --- a/packages/drizzle/src/updateMany.ts +++ b/packages/drizzle/src/updateMany.ts @@ -3,9 +3,8 @@ import type { UpdateMany } from 'payload' import toSnakeCase from 'to-snake-case' -import type { ChainedMethods, DrizzleAdapter } from './types.js' +import type { DrizzleAdapter } from './types.js' -import { chainMethods } from './find/chainMethods.js' import buildQuery from './queries/buildQuery.js' import { selectDistinct } from './queries/selectDistinct.js' import { upsertRow } from './upsertRow/index.js' @@ -45,16 +44,10 @@ export const updateMany: UpdateMany = async function updateMany( const selectDistinctResult = await selectDistinct({ adapter: this, - chainedMethods: orderBy - ? [ - { - args: [() => orderBy.map(({ column, order }) => order(column))], - method: 'orderBy', - }, - ] - : [], db, joins, + query: ({ query }) => + orderBy ? query.orderBy(() => orderBy.map(({ column, order }) => order(column))) : query, selectFields, tableName, where, @@ -69,28 +62,17 @@ export const updateMany: UpdateMany = async function updateMany( const table = this.tables[tableName] - const query = _db.select({ id: table.id }).from(table).where(where) - - const chainedMethods: ChainedMethods = [] + let query = _db.select({ id: table.id }).from(table).where(where).$dynamic() if (typeof limit === 'number' && limit > 0) { - chainedMethods.push({ - args: [limit], - method: 'limit', - }) + query = query.limit(limit) } if (orderBy) { - chainedMethods.push({ - args: [() => orderBy.map(({ column, order }) => order(column))], - method: 'orderBy', - }) + query = query.orderBy(() => orderBy.map(({ column, order }) => order(column))) } - const docsToUpdate = await chainMethods({ - methods: chainedMethods, - query, - }) + const docsToUpdate = await query idsToUpdate = docsToUpdate?.map((doc) => doc.id) } diff --git a/packages/drizzle/src/updateOne.ts b/packages/drizzle/src/updateOne.ts index 9119e7f8c3..e269961c24 100644 --- a/packages/drizzle/src/updateOne.ts +++ b/packages/drizzle/src/updateOne.ts @@ -41,9 +41,9 @@ export const updateOne: UpdateOne = async function updateOne( // selectDistinct will only return if there are joins const selectDistinctResult = await selectDistinct({ adapter: this, - chainedMethods: [{ args: [1], method: 'limit' }], db, joins, + query: ({ query }) => query.limit(1), selectFields, tableName, where,