From 055cc4ef1258ae073a06ad66cb3cb05f95f31bd3 Mon Sep 17 00:00:00 2001 From: Sasha <64744993+r1tsuu@users.noreply.github.com> Date: Thu, 10 Jul 2025 16:49:12 +0300 Subject: [PATCH] perf(db-postgres): simplify `db.updateOne` to a single DB call with if the passed data doesn't include nested fields (#13060) In case, if `payload.db.updateOne` received simple data, meaning no: * Arrays / Blocks * Localized Fields * `hasMany: true` text / select / number / relationship fields * relationship fields with `relationTo` as an array This PR simplifies the logic to a single SQL `set` call. No any extra (useless) steps with rewriting all the arrays / blocks / localized tables even if there were no any changes to them. However, it's good to note that `payload.update` (not `payload.db.updateOne`) as for now passes all the previous data as well, so this change won't have any effect unless you're using `payload.db.updateOne` directly (or for our internal logic that uses it), in the future a separate PR with optimization for `payload.update` as well may be implemented. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1210710489889576 --- packages/drizzle/src/updateOne.ts | 118 ++++++++++++++++-- test/database/config.ts | 14 +++ test/database/int.spec.ts | 31 ++++- test/database/payload-types.ts | 16 +++ ...50624_214621.json => 20250707_123508.json} | 86 ++++++++++++- ...{20250624_214621.ts => 20250707_123508.ts} | 18 ++- .../up-down-migration/migrations/index.ts | 8 +- 7 files changed, 272 insertions(+), 19 deletions(-) rename test/database/up-down-migration/migrations/{20250624_214621.json => 20250707_123508.json} (88%) rename test/database/up-down-migration/migrations/{20250624_214621.ts => 20250707_123508.ts} (86%) diff --git a/packages/drizzle/src/updateOne.ts b/packages/drizzle/src/updateOne.ts index 8fddd9378..ef451c943 100644 --- a/packages/drizzle/src/updateOne.ts +++ b/packages/drizzle/src/updateOne.ts @@ -1,15 +1,67 @@ import type { LibSQLDatabase } from 'drizzle-orm/libsql' -import type { UpdateOne } from 'payload' +import type { FlattenedField, UpdateOne } from 'payload' +import { eq } from 'drizzle-orm' import toSnakeCase from 'to-snake-case' import type { DrizzleAdapter } from './types.js' +import { buildFindManyArgs } from './find/buildFindManyArgs.js' import { buildQuery } from './queries/buildQuery.js' import { selectDistinct } from './queries/selectDistinct.js' +import { transform } from './transform/read/index.js' +import { transformForWrite } from './transform/write/index.js' import { upsertRow } from './upsertRow/index.js' import { getTransaction } from './utilities/getTransaction.js' +/** + * Checks whether we should use the upsertRow function for the passed data and otherwise use a simple SQL SET call. + * We need to use upsertRow only when the data has arrays, blocks, hasMany select/text/number, localized fields, complex relationships. + */ +const shouldUseUpsertRow = ({ + data, + fields, +}: { + data: Record + fields: FlattenedField[] +}) => { + for (const key in data) { + const value = data[key] + const field = fields.find((each) => each.name === key) + + if (!field) { + continue + } + + if ( + field.type === 'array' || + field.type === 'blocks' || + ((field.type === 'text' || + field.type === 'relationship' || + field.type === 'upload' || + field.type === 'select' || + field.type === 'number') && + field.hasMany) || + ((field.type === 'relationship' || field.type === 'upload') && + Array.isArray(field.relationTo)) || + field.localized + ) { + return true + } + + if ( + (field.type === 'group' || field.type === 'tab') && + value && + typeof value === 'object' && + shouldUseUpsertRow({ data: value as Record, fields: field.flattenedFields }) + ) { + return true + } + } + + return false +} + export const updateOne: UpdateOne = async function updateOne( this: DrizzleAdapter, { @@ -74,23 +126,71 @@ export const updateOne: UpdateOne = async function updateOne( return null } - const result = await upsertRow({ - id: idToUpdate, + if (!idToUpdate || shouldUseUpsertRow({ data, fields: collection.flattenedFields })) { + const result = await upsertRow({ + id: idToUpdate, + adapter: this, + data, + db, + fields: collection.flattenedFields, + ignoreResult: returning === false, + joinQuery, + operation: 'update', + req, + select, + tableName, + }) + + if (returning === false) { + return null + } + + return result + } + + const { row } = transformForWrite({ adapter: this, data, - db, fields: collection.flattenedFields, - ignoreResult: returning === false, - joinQuery, - operation: 'update', - req, - select, tableName, }) + const drizzle = db as LibSQLDatabase + await drizzle + .update(this.tables[tableName]) + .set(row) + // TODO: we can skip fetching idToUpdate here with using the incoming where + .where(eq(this.tables[tableName].id, idToUpdate)) + if (returning === false) { return null } + const findManyArgs = buildFindManyArgs({ + adapter: this, + depth: 0, + fields: collection.flattenedFields, + joinQuery: false, + select, + tableName, + }) + + findManyArgs.where = eq(this.tables[tableName].id, idToUpdate) + + const doc = await db.query[tableName].findFirst(findManyArgs) + + // ////////////////////////////////// + // TRANSFORM DATA + // ////////////////////////////////// + + const result = transform({ + adapter: this, + config: this.payload.config, + data: doc, + fields: collection.flattenedFields, + joinQuery: false, + tableName, + }) + return result } diff --git a/test/database/config.ts b/test/database/config.ts index 2e6c26972..1027491ea 100644 --- a/test/database/config.ts +++ b/test/database/config.ts @@ -223,6 +223,20 @@ export default buildConfigWithDefaults({ }, ], }, + { + type: 'group', + name: 'group', + fields: [{ name: 'text', type: 'text' }], + }, + { + type: 'tabs', + tabs: [ + { + name: 'tab', + fields: [{ name: 'text', type: 'text' }], + }, + ], + }, ], hooks: { beforeOperation: [ diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index 52c1969c5..34bcc13da 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -7,9 +7,7 @@ import { migrateRelationshipsV2_V3, migrateVersionsV1_V2, } from '@payloadcms/db-mongodb/migration-utils' -import { objectToFrontmatter } from '@payloadcms/richtext-lexical' import { randomUUID } from 'crypto' -import { type Table } from 'drizzle-orm' import * as drizzlePg from 'drizzle-orm/pg-core' import * as drizzleSqlite from 'drizzle-orm/sqlite-core' import fs from 'fs' @@ -2809,6 +2807,35 @@ describe('database', () => { } }) + it('should update simple', async () => { + const post = await payload.create({ + collection: 'posts', + data: { + text: 'other text (should not be nuked)', + title: 'hello', + group: { text: 'in group' }, + tab: { text: 'in tab' }, + arrayWithIDs: [{ text: 'some text' }], + }, + }) + const res = await payload.db.updateOne({ + where: { id: { equals: post.id } }, + data: { + title: 'hello updated', + group: { text: 'in group updated' }, + tab: { text: 'in tab updated' }, + }, + collection: 'posts', + }) + + expect(res.title).toBe('hello updated') + expect(res.text).toBe('other text (should not be nuked)') + expect(res.group.text).toBe('in group updated') + expect(res.tab.text).toBe('in tab updated') + expect(res.arrayWithIDs).toHaveLength(1) + expect(res.arrayWithIDs[0].text).toBe('some text') + }) + it('should support x3 nesting blocks', async () => { const res = await payload.create({ collection: 'posts', diff --git a/test/database/payload-types.ts b/test/database/payload-types.ts index 0975fc1b5..d1f52cb4a 100644 --- a/test/database/payload-types.ts +++ b/test/database/payload-types.ts @@ -232,6 +232,12 @@ export interface Post { blockType: 'block-first'; }[] | null; + group?: { + text?: string | null; + }; + tab?: { + text?: string | null; + }; updatedAt: string; createdAt: string; } @@ -804,6 +810,16 @@ export interface PostsSelect { blockName?: T; }; }; + group?: + | T + | { + text?: T; + }; + tab?: + | T + | { + text?: T; + }; updatedAt?: T; createdAt?: T; } diff --git a/test/database/up-down-migration/migrations/20250624_214621.json b/test/database/up-down-migration/migrations/20250707_123508.json similarity index 88% rename from test/database/up-down-migration/migrations/20250624_214621.json rename to test/database/up-down-migration/migrations/20250707_123508.json index 3e1e61171..f54134521 100644 --- a/test/database/up-down-migration/migrations/20250624_214621.json +++ b/test/database/up-down-migration/migrations/20250707_123508.json @@ -1,9 +1,93 @@ { - "id": "a3dd8ca0-5e09-407b-9178-e0ff7f15da59", + "id": "bf183b76-944c-4e83-bd58-4aa993885106", "prevId": "00000000-0000-0000-0000-000000000000", "version": "7", "dialect": "postgresql", "tables": { + "public.users_sessions": { + "name": "users_sessions", + "schema": "", + "columns": { + "_order": { + "name": "_order", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "_parent_id": { + "name": "_parent_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "id": { + "name": "id", + "type": "varchar", + "primaryKey": true, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp(3) with time zone", + "primaryKey": false, + "notNull": false + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp(3) with time zone", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "users_sessions_order_idx": { + "name": "users_sessions_order_idx", + "columns": [ + { + "expression": "_order", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "users_sessions_parent_id_idx": { + "name": "users_sessions_parent_id_idx", + "columns": [ + { + "expression": "_parent_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "users_sessions_parent_id_fk": { + "name": "users_sessions_parent_id_fk", + "tableFrom": "users_sessions", + "tableTo": "users", + "columnsFrom": ["_parent_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.users": { "name": "users", "schema": "", diff --git a/test/database/up-down-migration/migrations/20250624_214621.ts b/test/database/up-down-migration/migrations/20250707_123508.ts similarity index 86% rename from test/database/up-down-migration/migrations/20250624_214621.ts rename to test/database/up-down-migration/migrations/20250707_123508.ts index 0f65d8f59..098ecd2a0 100644 --- a/test/database/up-down-migration/migrations/20250624_214621.ts +++ b/test/database/up-down-migration/migrations/20250707_123508.ts @@ -1,10 +1,18 @@ -import type { MigrateDownArgs, MigrateUpArgs } from '@payloadcms/db-postgres' +import type { MigrateDownArgs, MigrateUpArgs} from '@payloadcms/db-postgres'; import { sql } from '@payloadcms/db-postgres' export async function up({ db, payload, req }: MigrateUpArgs): Promise { await db.execute(sql` - CREATE TABLE "users" ( + CREATE TABLE "users_sessions" ( + "_order" integer NOT NULL, + "_parent_id" integer NOT NULL, + "id" varchar PRIMARY KEY NOT NULL, + "created_at" timestamp(3) with time zone, + "expires_at" timestamp(3) with time zone NOT NULL + ); + + CREATE TABLE "users" ( "id" serial PRIMARY KEY NOT NULL, "updated_at" timestamp(3) with time zone DEFAULT now() NOT NULL, "created_at" timestamp(3) with time zone DEFAULT now() NOT NULL, @@ -56,10 +64,13 @@ export async function up({ db, payload, req }: MigrateUpArgs): Promise { "created_at" timestamp(3) with time zone DEFAULT now() NOT NULL ); + ALTER TABLE "users_sessions" ADD CONSTRAINT "users_sessions_parent_id_fk" FOREIGN KEY ("_parent_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action; ALTER TABLE "payload_locked_documents_rels" ADD CONSTRAINT "payload_locked_documents_rels_parent_fk" FOREIGN KEY ("parent_id") REFERENCES "public"."payload_locked_documents"("id") ON DELETE cascade ON UPDATE no action; ALTER TABLE "payload_locked_documents_rels" ADD CONSTRAINT "payload_locked_documents_rels_users_fk" FOREIGN KEY ("users_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action; ALTER TABLE "payload_preferences_rels" ADD CONSTRAINT "payload_preferences_rels_parent_fk" FOREIGN KEY ("parent_id") REFERENCES "public"."payload_preferences"("id") ON DELETE cascade ON UPDATE no action; ALTER TABLE "payload_preferences_rels" ADD CONSTRAINT "payload_preferences_rels_users_fk" FOREIGN KEY ("users_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action; + CREATE INDEX "users_sessions_order_idx" ON "users_sessions" USING btree ("_order"); + CREATE INDEX "users_sessions_parent_id_idx" ON "users_sessions" USING btree ("_parent_id"); CREATE INDEX "users_updated_at_idx" ON "users" USING btree ("updated_at"); CREATE INDEX "users_created_at_idx" ON "users" USING btree ("created_at"); CREATE UNIQUE INDEX "users_email_idx" ON "users" USING btree ("email"); @@ -83,7 +94,8 @@ export async function up({ db, payload, req }: MigrateUpArgs): Promise { export async function down({ db, payload, req }: MigrateDownArgs): Promise { await db.execute(sql` - DROP TABLE "users" CASCADE; + DROP TABLE "users_sessions" CASCADE; + DROP TABLE "users" CASCADE; DROP TABLE "payload_locked_documents" CASCADE; DROP TABLE "payload_locked_documents_rels" CASCADE; DROP TABLE "payload_preferences" CASCADE; diff --git a/test/database/up-down-migration/migrations/index.ts b/test/database/up-down-migration/migrations/index.ts index 91d190b45..0c0f71044 100644 --- a/test/database/up-down-migration/migrations/index.ts +++ b/test/database/up-down-migration/migrations/index.ts @@ -1,9 +1,9 @@ -import * as migration_20250624_214621 from './20250624_214621.js' +import * as migration_20250707_123508 from './20250707_123508.js' export const migrations = [ { - up: migration_20250624_214621.up, - down: migration_20250624_214621.down, - name: '20250624_214621', + up: migration_20250707_123508.up, + down: migration_20250707_123508.down, + name: '20250707_123508', }, ]