From e737277685eaad7a6437065ece5fd96a835ad56c Mon Sep 17 00:00:00 2001 From: Matija Boban Date: Thu, 30 Jul 2026 10:42:25 -0700 Subject: [PATCH 1/3] perf(orm): reuse QueryNameMapper across derived executors and clients QueryNameMapper is derived purely from the client's $schema and $options, but it was rebuilt for every derived executor and every derived client. Building it is O(models x fields), plus an O(models x relations) pass on postgres. A $transaction paid that twice per scope, via two independent paths: 1. ZenStackQueryExecutor.withConnectionProvider 2. new ClientImpl(...) with a baseClient but no executor, which interactiveTransaction and sequentialTransaction both take The executor now accepts an optional mapper and threads its own through all five derive sites. ClientImpl passes the base client's mapper when the derived client has the same schema and options by identity - so $use / $unuse / $setOptions, which derive with a new options object and therefore a different dialect, still build their own. Sharing one instance is safe because the transformer holds no cross-transform state: its only mutable field, `scopes`, is pushed and popped in `finally` within a single synchronous traversal, and the class contains no async, await or Promise, so two transforms cannot interleave on a single-threaded runtime. Measured on a 1067-model / 11938-field postgres schema, against dev: paginated read + follow-up, no transaction 2.61 ms -> 2.18 ms same pair inside $transaction 12.06 ms -> 3.86 ms one connection scope in isolation 3.16 ms -> 0.07 ms Refs #2773 --- packages/orm/src/client/client-impl.ts | 17 ++++ .../executor/zenstack-query-executor.ts | 28 ++++-- tests/regression/test/issue-2773.test.ts | 85 +++++++++++++++++++ 3 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 tests/regression/test/issue-2773.test.ts diff --git a/packages/orm/src/client/client-impl.ts b/packages/orm/src/client/client-impl.ts index 0fb4e4dbd..92494742a 100644 --- a/packages/orm/src/client/client-impl.ts +++ b/packages/orm/src/client/client-impl.ts @@ -50,6 +50,13 @@ type ExtResultFieldDef = { compute: (data: Record) => unknown; }; +/** + * Returns the name mapper held by a ZenStack executor, or undefined for a plain kysely one. + */ +function getExecutorNameMapper(executor: QueryExecutor | undefined) { + return executor instanceof ZenStackQueryExecutor ? executor.getNameMapper() : undefined; +} + /** * ZenStack ORM client. */ @@ -99,6 +106,16 @@ export class ClientImpl { baseClient.kyselyProps.dialect.createQueryCompiler(), baseClient.kyselyProps.dialect.createAdapter(), new DefaultConnectionProvider(baseClient.kyselyProps.driver), + [], + false, + // A name mapper is derived purely from `$schema` and `$options`, so it can be + // reused when neither changed - which is the case for derived clients like the + // one `$transaction` creates. Rebuilding it is O(models x fields). See #2773. + // Deliberately an identity check: `$use`/`$setOptions` and friends pass a new + // options object, and the mapper's dialect is built from those options. + baseClient.$schema === schema && baseClient.$options === options + ? getExecutorNameMapper(baseClient.kyselyProps.executor) + : undefined, ), }; this.kyselyRaw = baseClient.kyselyRaw; diff --git a/packages/orm/src/client/executor/zenstack-query-executor.ts b/packages/orm/src/client/executor/zenstack-query-executor.ts index ed4f6f6b1..a2ffed714 100644 --- a/packages/orm/src/client/executor/zenstack-query-executor.ts +++ b/packages/orm/src/client/executor/zenstack-query-executor.ts @@ -86,19 +86,32 @@ export class ZenStackQueryExecutor extends DefaultQueryExecutor { private readonly connectionProvider: ConnectionProvider, plugins: KyselyPlugin[] = [], private suppressMutationHooks: boolean = false, + nameMapper?: QueryNameMapper, ) { super(compiler, adapter, connectionProvider, plugins); - if ( - client.$schema.provider.type === 'postgresql' || // postgres queries need to be schema-qualified + // A `QueryNameMapper` is derived purely from the client's `$schema` and `$options`, and building + // it is O(models x fields) (plus an O(models x relations) pass for postgres). Reuse the one from + // the executor/client we're derived from when it was built from the same schema and options, + // otherwise every derived executor rebuilds whole-schema state. See issue #2773. + this.nameMapper = + nameMapper ?? + (client.$schema.provider.type === 'postgresql' || // postgres queries need to be schema-qualified this.schemaHasMappedNames(client.$schema) - ) { - this.nameMapper = new QueryNameMapper(client as unknown as ClientContract); - } + ? new QueryNameMapper(client as unknown as ClientContract) + : undefined); this.dialect = getCrudDialect(client.$schema, client.$options); } + /** + * The name mapper built for this executor's schema, if the schema needs one. Exposed so that + * derived clients built from the same schema and options can reuse it instead of rebuilding it. + */ + getNameMapper() { + return this.nameMapper; + } + private schemaHasMappedNames(schema: SchemaDef) { const hasMapAttr = (decl: ModelDef | TypeDefDef) => { if (decl.attributes?.some((attr) => attr.name === '@@map')) { @@ -770,6 +783,7 @@ In such cases, ZenStack cannot reliably determine the IDs of the mutated entitie this.connectionProvider, [...this.plugins, plugin], this.suppressMutationHooks, + this.nameMapper, ); } @@ -782,6 +796,7 @@ In such cases, ZenStack cannot reliably determine the IDs of the mutated entitie this.connectionProvider, [...this.plugins, ...plugins], this.suppressMutationHooks, + this.nameMapper, ); } @@ -794,6 +809,7 @@ In such cases, ZenStack cannot reliably determine the IDs of the mutated entitie this.connectionProvider, [plugin, ...this.plugins], this.suppressMutationHooks, + this.nameMapper, ); } @@ -806,6 +822,7 @@ In such cases, ZenStack cannot reliably determine the IDs of the mutated entitie this.connectionProvider, [], this.suppressMutationHooks, + this.nameMapper, ); } @@ -818,6 +835,7 @@ In such cases, ZenStack cannot reliably determine the IDs of the mutated entitie connectionProvider, this.plugins as KyselyPlugin[], this.suppressMutationHooks, + this.nameMapper, ); // replace client with a new one associated with the new executor newExecutor.client = this.client.withExecutor(newExecutor); diff --git a/tests/regression/test/issue-2773.test.ts b/tests/regression/test/issue-2773.test.ts new file mode 100644 index 000000000..94629036c --- /dev/null +++ b/tests/regression/test/issue-2773.test.ts @@ -0,0 +1,85 @@ +import { createTestClient } from '@zenstackhq/testtools'; +import { describe, expect, it } from 'vitest'; + +// https://github.com/zenstackhq/zenstack/issues/2773 +// +// `QueryNameMapper` is derived purely from the client's `$schema` and `$options`, but it was rebuilt +// for every derived executor/client. Building it is O(models x fields), plus an O(models x relations) +// pass on postgres, so on a large schema every `$transaction` paid that cost twice - once when +// `$transaction` derives a client, and once when the query derives a connection-scoped executor. +// +// These assert the invariant (the mapper is not rebuilt) rather than elapsed time, which would be +// both flaky and only an indirect proxy for it. + +const schema = ` +model Post { + id Int @id @default(autoincrement()) + title String @map("post_title") + + @@map("posts_table") +} +`; + +// Note: inside a transaction `$qb.getExecutor()` is kysely's own wrapping executor, so we read the +// ZenStack executor the client itself was built with. We read the underlying field rather than the +// accessor so that these assertions fail on a mapper-identity mismatch - the actual defect - rather +// than on a missing method. +function nameMapperOf(client: any) { + return client.kyselyProps.executor.nameMapper; +} + +describe('Regression for issue #2773', () => { + it('reuses the name mapper for the client derived by $transaction', async () => { + const db = await createTestClient(schema, { provider: 'postgresql' }); + const mapper = nameMapperOf(db); + expect(mapper).toBeDefined(); + + await db.$transaction(async (tx: any) => { + expect(nameMapperOf(tx)).toBe(mapper); + }); + }); + + it('reuses the name mapper for connection-scoped and plugin-derived executors', async () => { + const db = await createTestClient(schema, { provider: 'postgresql' }); + const executor = (db as any).kyselyProps.executor; + const mapper = executor.nameMapper; + expect(mapper).toBeDefined(); + + expect(executor.withoutPlugins().nameMapper).toBe(mapper); + expect(executor.withPlugin({}).nameMapper).toBe(mapper); + expect(executor.withPluginAtFront({}).nameMapper).toBe(mapper); + expect(executor.withPlugins([{}]).nameMapper).toBe(mapper); + expect(executor.withConnectionProvider(executor.connectionProvider).nameMapper).toBe(mapper); + }); + + it('still builds a mapper for a client derived with different options', async () => { + const db = await createTestClient(schema, { provider: 'postgresql' }); + const mapper = nameMapperOf(db); + + // `$use` derives a client with a NEW options object, and the mapper's dialect is built from + // options - so this one must not inherit the existing mapper. + const derived = db.$use({ id: 'noop', name: 'noop' } as any); + expect(nameMapperOf(derived)).toBeDefined(); + expect(nameMapperOf(derived)).not.toBe(mapper); + }); + + it('applies @@map and @map identically inside and outside a transaction', async () => { + const db = await createTestClient(schema, { provider: 'postgresql' }); + await db.post.create({ data: { title: 'hello' } }); + + const outside = await db.post.findMany(); + expect(outside).toHaveLength(1); + expect(outside[0].title).toBe('hello'); + + const inside = await db.$transaction(async (tx: any) => tx.post.findMany()); + expect(inside).toEqual(outside); + + // the mapped table/column really are what the mapper produced + const raw = await db.$qb + .selectFrom('posts_table' as any) + .selectAll() + .execute(); + expect(raw).toHaveLength(1); + expect((raw[0] as any).post_title).toBe('hello'); + }); +}); From 531bac40fa51eb4f897f6d21e090a9eadf02943f Mon Sep 17 00:00:00 2001 From: Matija Boban Date: Thu, 30 Jul 2026 12:53:38 -0700 Subject: [PATCH 2/3] perf(orm): memoize the schema-has-mapped-names check per schema Deciding whether a schema needs a QueryNameMapper walks every model, type def and field. It is asked on every query-executor construction, and it is not short-circuited on non-postgres providers - so a sqlite or mysql schema with no @@map/@map anywhere paid the full walk for each derived executor, including twice per transaction scope. The answer is immutable for a schema, so it moves next to the other structural lookups in query-utils and is memoized through the existing per-schema WeakMap cache. On a synthetic 1067-model / 11737-field schema with no mapped names, the walk costs 812 us; after the first call it is 0.02 us. Refs #2773 --- .../executor/zenstack-query-executor.ts | 17 ++--------- packages/orm/src/client/query-utils.ts | 30 ++++++++++++++++++- tests/regression/test/issue-2773.test.ts | 25 ++++++++++++++++ 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/packages/orm/src/client/executor/zenstack-query-executor.ts b/packages/orm/src/client/executor/zenstack-query-executor.ts index a2ffed714..b92f4da84 100644 --- a/packages/orm/src/client/executor/zenstack-query-executor.ts +++ b/packages/orm/src/client/executor/zenstack-query-executor.ts @@ -1,5 +1,5 @@ import { invariant } from '@zenstackhq/common-helpers'; -import type { ModelDef, SchemaDef, TypeDefDef } from '@zenstackhq/schema'; +import type { SchemaDef } from '@zenstackhq/schema'; import type { QueryId } from 'kysely'; import { AndNode, @@ -37,7 +37,7 @@ import { getCrudDialect } from '../crud/dialects'; import type { BaseCrudDialect } from '../crud/dialects/base-dialect'; import { createDBQueryError, createInternalError, ORMError } from '../errors'; import type { AfterEntityMutationCallback, OnKyselyQueryCallback } from '../plugin'; -import { requireIdFields, stripAlias } from '../query-utils'; +import { requireIdFields, schemaHasMappedNames, stripAlias } from '../query-utils'; import { QueryNameMapper } from './name-mapper'; import { TempAliasTransformer } from './temp-alias-transformer'; import type { ZenStackDriver } from './zenstack-driver'; @@ -97,7 +97,7 @@ export class ZenStackQueryExecutor extends DefaultQueryExecutor { this.nameMapper = nameMapper ?? (client.$schema.provider.type === 'postgresql' || // postgres queries need to be schema-qualified - this.schemaHasMappedNames(client.$schema) + schemaHasMappedNames(client.$schema) ? new QueryNameMapper(client as unknown as ClientContract) : undefined); @@ -112,17 +112,6 @@ export class ZenStackQueryExecutor extends DefaultQueryExecutor { return this.nameMapper; } - private schemaHasMappedNames(schema: SchemaDef) { - const hasMapAttr = (decl: ModelDef | TypeDefDef) => { - if (decl.attributes?.some((attr) => attr.name === '@@map')) { - return true; - } - return Object.values(decl.fields).some((field) => field.attributes?.some((attr) => attr.name === '@map')); - }; - - return Object.values(schema.models).some(hasMapAttr) || Object.values(schema.typeDefs ?? []).some(hasMapAttr); - } - private get kysely() { return this.client.$qb; } diff --git a/packages/orm/src/client/query-utils.ts b/packages/orm/src/client/query-utils.ts index 5a4b146b4..50a8bf75d 100644 --- a/packages/orm/src/client/query-utils.ts +++ b/packages/orm/src/client/query-utils.ts @@ -1,5 +1,12 @@ import { invariant } from '@zenstackhq/common-helpers'; -import { ExpressionUtils, type FieldDef, type GetModels, type ModelDef, type SchemaDef } from '@zenstackhq/schema'; +import { + ExpressionUtils, + type FieldDef, + type GetModels, + type ModelDef, + type SchemaDef, + type TypeDefDef, +} from '@zenstackhq/schema'; import { AliasNode, ColumnNode, @@ -30,6 +37,7 @@ interface SchemaLookupCache { model: Map; m2mRelation: Map>; m2mJoinTable?: Map; + hasMappedNames?: boolean; } const schemaLookupCache = new WeakMap(); @@ -58,6 +66,26 @@ export function getTypeDef(schema: SchemaDef, type: string) { return schema.typeDefs?.[type]; } +/** + * Whether any model, type def, or field in the schema carries `@@map`/`@map`. Answering it walks + * every model and field, and it is asked once per query-executor construction, so the (immutable) + * answer is memoized per schema alongside the other structural lookups. See issue #2773. + */ +export function schemaHasMappedNames(schema: SchemaDef) { + const cache = getSchemaLookupCache(schema); + if (cache.hasMappedNames === undefined) { + const hasMapAttr = (decl: ModelDef | TypeDefDef) => { + if (decl.attributes?.some((attr) => attr.name === '@@map')) { + return true; + } + return Object.values(decl.fields).some((field) => field.attributes?.some((attr) => attr.name === '@map')); + }; + cache.hasMappedNames = + Object.values(schema.models).some(hasMapAttr) || Object.values(schema.typeDefs ?? []).some(hasMapAttr); + } + return cache.hasMappedNames; +} + export function requireModel(schema: SchemaDef, model: string) { const modelDef = getModel(schema, model); if (!modelDef) { diff --git a/tests/regression/test/issue-2773.test.ts b/tests/regression/test/issue-2773.test.ts index 94629036c..9df3c0b40 100644 --- a/tests/regression/test/issue-2773.test.ts +++ b/tests/regression/test/issue-2773.test.ts @@ -63,6 +63,31 @@ describe('Regression for issue #2773', () => { expect(nameMapperOf(derived)).not.toBe(mapper); }); + it('keeps working when the schema needs no mapper at all', async () => { + // sqlite + no @@map/@map means nameMapper stays undefined. Deciding that walks every model + // and field, and it is asked on every executor construction, so it is memoized per schema - + // this guards the path where nothing is threaded through. + const db = await createTestClient( + ` +model Item { + id Int @id @default(autoincrement()) + name String +} + `, + { provider: 'sqlite' }, + ) + expect(nameMapperOf(db)).toBeUndefined(); + + await db.item.create({ data: { name: 'a' } }); + const outside = await db.item.findMany(); + expect(outside).toHaveLength(1); + + await db.$transaction(async (tx: any) => { + expect(nameMapperOf(tx)).toBeUndefined(); + expect(await tx.item.findMany()).toHaveLength(1); + }); + }); + it('applies @@map and @map identically inside and outside a transaction', async () => { const db = await createTestClient(schema, { provider: 'postgresql' }); await db.post.create({ data: { title: 'hello' } }); From 66ede0ceecd30ce7e6d89e5ce102aca7ac0747a0 Mon Sep 17 00:00:00 2001 From: Matija Boban Date: Mon, 3 Aug 2026 16:55:16 -0700 Subject: [PATCH 3/3] fix(orm): include enums when deciding whether a schema has mapped names schemaHasMappedNames checked models and type defs only. Enums carry name mapping too - @@map on the enum and @map on its members - so a schema whose only mapped name is on an enum got no QueryNameMapper on any provider other than postgres, where the mapper is built unconditionally and masked it. EnumDef.fields is optional where ModelDef.fields and TypeDefDef.fields are required, hence the ?? {} guard. Per review feedback from @ymc9 on #2777. --- packages/orm/src/client/query-utils.ts | 13 ++++-- tests/regression/test/issue-2773.test.ts | 53 +++++++++++++++++++++++- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/packages/orm/src/client/query-utils.ts b/packages/orm/src/client/query-utils.ts index 50a8bf75d..7941b7faf 100644 --- a/packages/orm/src/client/query-utils.ts +++ b/packages/orm/src/client/query-utils.ts @@ -1,5 +1,6 @@ import { invariant } from '@zenstackhq/common-helpers'; import { + type EnumDef, ExpressionUtils, type FieldDef, type GetModels, @@ -74,14 +75,20 @@ export function getTypeDef(schema: SchemaDef, type: string) { export function schemaHasMappedNames(schema: SchemaDef) { const cache = getSchemaLookupCache(schema); if (cache.hasMappedNames === undefined) { - const hasMapAttr = (decl: ModelDef | TypeDefDef) => { + // `fields` is optional on `EnumDef` (required on the other two), hence the `?? {}`. + const hasMapAttr = (decl: ModelDef | TypeDefDef | EnumDef) => { if (decl.attributes?.some((attr) => attr.name === '@@map')) { return true; } - return Object.values(decl.fields).some((field) => field.attributes?.some((attr) => attr.name === '@map')); + return Object.values(decl.fields ?? {}).some((field) => + field.attributes?.some((attr) => attr.name === '@map'), + ); }; cache.hasMappedNames = - Object.values(schema.models).some(hasMapAttr) || Object.values(schema.typeDefs ?? []).some(hasMapAttr); + Object.values(schema.models).some(hasMapAttr) || + Object.values(schema.typeDefs ?? {}).some(hasMapAttr) || + // Enums carry name mapping too — `@@map` on the enum and `@map` on its members. + Object.values(schema.enums ?? {}).some(hasMapAttr); } return cache.hasMappedNames; } diff --git a/tests/regression/test/issue-2773.test.ts b/tests/regression/test/issue-2773.test.ts index 9df3c0b40..f16c282a6 100644 --- a/tests/regression/test/issue-2773.test.ts +++ b/tests/regression/test/issue-2773.test.ts @@ -75,7 +75,7 @@ model Item { } `, { provider: 'sqlite' }, - ) + ); expect(nameMapperOf(db)).toBeUndefined(); await db.item.create({ data: { name: 'a' } }); @@ -88,6 +88,57 @@ model Item { }); }); + // Enums carry name mapping too, and `schemaHasMappedNames` originally checked only models + // and type defs. On postgres the mapper is built unconditionally, which masked it; on any + // other provider a schema whose ONLY mapped name is on an enum got no mapper at all. + it('builds a mapper for a schema whose only mapped name is on an enum', async () => { + const db = await createTestClient( + ` +enum Status { + ACTIVE + ARCHIVED + + @@map('status_enum') +} + +model Item { + id Int @id @default(autoincrement()) + name String + status Status @default(ACTIVE) +} + `, + { provider: 'sqlite' }, + ); + + expect(nameMapperOf(db)).toBeDefined(); + + await db.item.create({ data: { name: 'a' } }); + expect(await db.item.findMany()).toHaveLength(1); + }); + + it('builds a mapper for a schema whose only mapped name is on an enum MEMBER', async () => { + const db = await createTestClient( + ` +enum Status { + ACTIVE @map('is_active') + ARCHIVED +} + +model Item { + id Int @id @default(autoincrement()) + name String + status Status @default(ACTIVE) +} + `, + { provider: 'sqlite' }, + ); + + expect(nameMapperOf(db)).toBeDefined(); + + await db.item.create({ data: { name: 'a' } }); + expect(await db.item.findMany()).toHaveLength(1); + }); + it('applies @@map and @map identically inside and outside a transaction', async () => { const db = await createTestClient(schema, { provider: 'postgresql' }); await db.post.create({ data: { title: 'hello' } });