From 7475b5f16fdd27030003bb69748cc92d259fd65a Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Thu, 9 Jul 2026 15:47:54 +0200 Subject: [PATCH] perf(upsert) - tighten createMany upsert candidate lookup to avoid broad OR scans (#22721) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes a performance-correctness bug in the createMany upsert path where the existing-record lookup built an overly broad WHERE clause, causing full table scans and multi-second latency on bulk upserts. Reported via Sentry: a 100-record create*(upsert: true) request on _sdWorkspace completed with HTTP 200 but took ~14s. The candidate-lookup SELECT alone took ~6.7s because it fetched a huge superset of rows before matching in memory. ## Root cause buildWhereConditions created one IN(...) per conflicting column across the whole batch, and findExistingRecords OR-ed them together: WHERE "cbCustomerId" IN ($1..$100) OR "environment" IN ($101..$200) For a composite unique index (cbCustomerId, environment), this is semantically wrong: it OR's the columns instead of matching them as a tuple. Because environment is low-cardinality (prod/staging/dev/test), the second IN alone matched almost the entire table, forcing a Seq Scan and shipping the whole result set to the app for in-memory filtering. ## Fix buildWhereConditions now generates targeted lookup conditions: - Single-column unique keys collapse into one `column IN (distinctValues)` condition (instead of N OR-ed equalities). - Composite unique keys produce one `(colA = ? AND colB = ?)` condition per input record — the columns are ANDed as a tuple, and separate unique indexes are still OR-ed together. - Conditions are deduplicated (robust JSON-based key, no separator collisions) to avoid redundant OR branches. - Values are now typed as string | number | boolean instead of being implicitly coerced to string. ## Benchmark Reproduced the incident with a table mirroring _sdWorkspace: high-cardinality cbCustomerId + low-cardinality environment (4 values), composite unique index on (cbCustomerId, environment), 100-record upsert batch. EXPLAIN (ANALYZE, BUFFERS) on a warm 500k-row / 392 MB table: Metric | OLD (colA IN OR colB IN) | NEW (targeted) | Improvement ----------------------|--------------------------|----------------|------------ Scan type | Seq Scan (full table) | Index Scan | index vs full scan Rows returned to app | 500,000 | 100 | ~5,000x fewer Buffers touched | ~45,455 (~355 MB) | 400 (~3.2 MB) | ~110x fewer Execution time | 224 ms | 11.9 ms | ~19x faster Data shipped to app | ~322 MB | ~64 KB | ~5,000x less The cache-independent facts are the proof: the old query returned the entire table for a 100-record batch (the "overly broad record set" from the report), while the new query returns exactly the matching rows via the composite index. ## Test plan - [x] Unit tests for buildWhereConditions (single-column IN batching, nested paths, composite AND tuples, dedup incl. separator-collision safety, numeric values, mixed single-column + composite OR) — 22 passing across build-where-conditions, get-matching-record-id, get-value-from-path. - [x] Upsert integration suites pass (upsert + composite-unique-index-upsert, 10 tests). - [x] EXPLAIN ANALYZE benchmark confirms Index Scan and bounded row counts. Review in cubic --- .../types/conflicting-field-group.type.ts | 2 + .../build-where-conditions.util.spec.ts | 208 ++++++++++++++++-- .../utils/build-where-conditions.util.ts | 116 ++++++++-- .../utils/get-value-from-path.util.ts | 4 +- 4 files changed, 296 insertions(+), 34 deletions(-) diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type.ts index fb2d1741f5..315c46a24f 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type.ts @@ -1,3 +1,5 @@ +export type ConflictingFieldValue = string | number | boolean; + export type ConflictingProperty = { fullPath: string; column: string; diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/build-where-conditions.util.spec.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/build-where-conditions.util.spec.ts index 814cdc3eb4..4b71ced899 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/build-where-conditions.util.spec.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/build-where-conditions.util.spec.ts @@ -28,7 +28,7 @@ describe('buildWhereConditions', () => { expect(where).toEqual([]); }); - it('builds a single where condition for a flat field using all defined values', () => { + it('collapses a single-column group into one IN condition with defined values', () => { const groups: ConflictingFieldGroup[] = [ { baseFields: ['uniqueText'], @@ -41,16 +41,36 @@ describe('buildWhereConditions', () => { const where = buildWhereConditions(records, groups); expect(where).toHaveLength(1); - const condition = where[0]; - expect(Object.keys(condition)).toEqual(['uniqueText']); - - const operator = condition.uniqueText; + const operator = where[0].uniqueText; expect(operator.type.toLowerCase()).toBe('in'); expect(operator.value).toEqual(['alpha', 'beta']); }); + it('deduplicates values within a single-column IN condition', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['uniqueText'], + conflictingProperties: [ + { fullPath: 'uniqueText', column: 'uniqueText' }, + ], + }, + ]; + + const where = buildWhereConditions( + [ + { uniqueText: 'alpha' }, + { uniqueText: 'alpha' }, + { uniqueText: 'beta' }, + ], + groups, + ); + + expect(where).toHaveLength(1); + expect(where[0].uniqueText.value).toEqual(['alpha', 'beta']); + }); + it('skips adding a condition when all values for a field are undefined', () => { const groups: ConflictingFieldGroup[] = [ { @@ -66,7 +86,7 @@ describe('buildWhereConditions', () => { expect(where).toEqual([]); }); - it('builds conditions for nested paths', () => { + it('collapses a nested single-column path into one IN condition', () => { const groups: ConflictingFieldGroup[] = [ { baseFields: ['emailsField'], @@ -82,17 +102,14 @@ describe('buildWhereConditions', () => { const where = buildWhereConditions(records, groups); expect(where).toHaveLength(1); - const condition = where[0]; - expect(Object.keys(condition)).toEqual(['emailsFieldPrimaryEmail']); - - const operator = condition.emailsFieldPrimaryEmail; + const operator = where[0].emailsFieldPrimaryEmail; expect(operator.type.toLowerCase()).toBe('in'); expect(operator.value).toEqual(['alpha@example.com', 'beta@example.com']); }); - it('builds multiple conditions when multiple conflicting fields are provided', () => { + it('builds one IN condition per single-column conflicting field group', () => { const groups: ConflictingFieldGroup[] = [ { baseFields: ['uniqueText'], @@ -119,17 +136,170 @@ describe('buildWhereConditions', () => { 'emailsFieldPrimaryEmail', 'uniqueText', ]); + }); - const uniqueTextOperator = where.find((c) => 'uniqueText' in c)?.uniqueText; + it('preserves numeric values instead of coercing them to strings', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['externalId'], + conflictingProperties: [ + { fullPath: 'externalId', column: 'externalId' }, + ], + }, + ]; - const emailOperator = where.find( - (c) => 'emailsFieldPrimaryEmail' in c, - )?.emailsFieldPrimaryEmail; + const where = buildWhereConditions( + [{ externalId: 42 }, { externalId: 43 }], + groups, + ); - expect(uniqueTextOperator?.value).toEqual(['alpha', 'beta']); - expect(emailOperator?.value).toEqual([ - 'alpha@example.com', - 'beta@example.com', + expect(where).toHaveLength(1); + expect(where[0].externalId.value).toEqual([42, 43]); + }); + + it('builds composite group conditions with all properties ANDed together per record', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['customerId', 'environment'], + conflictingProperties: [ + { fullPath: 'customerId', column: 'customerId' }, + { fullPath: 'environment', column: 'environment' }, + ], + }, + ]; + + const where = buildWhereConditions( + [ + { customerId: 'customer-1', environment: 'prod' }, + { customerId: 'customer-2', environment: 'staging' }, + ], + groups, + ); + + expect(where).toHaveLength(2); + + expect(where).toEqual([ + { + customerId: expect.objectContaining({ value: 'customer-1' }), + environment: expect.objectContaining({ value: 'prod' }), + }, + { + customerId: expect.objectContaining({ value: 'customer-2' }), + environment: expect.objectContaining({ value: 'staging' }), + }, ]); + + where.forEach((condition) => { + expect(condition.customerId.type.toLowerCase()).toBe('equal'); + expect(condition.environment.type.toLowerCase()).toBe('equal'); + }); + }); + + it('skips a composite group for a record missing part of the key', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['customerId', 'environment'], + conflictingProperties: [ + { fullPath: 'customerId', column: 'customerId' }, + { fullPath: 'environment', column: 'environment' }, + ], + }, + ]; + + const where = buildWhereConditions( + [ + { customerId: 'customer-1', environment: 'prod' }, + { customerId: 'customer-2' }, + ], + groups, + ); + + expect(where).toHaveLength(1); + expect(where[0]).toEqual({ + customerId: expect.objectContaining({ value: 'customer-1' }), + environment: expect.objectContaining({ value: 'prod' }), + }); + }); + + it('deduplicates identical composite conditions across records', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['customerId', 'environment'], + conflictingProperties: [ + { fullPath: 'customerId', column: 'customerId' }, + { fullPath: 'environment', column: 'environment' }, + ], + }, + ]; + + const where = buildWhereConditions( + [ + { customerId: 'customer-1', environment: 'prod' }, + { customerId: 'customer-1', environment: 'prod' }, + ], + groups, + ); + + expect(where).toHaveLength(1); + + expect(where[0]).toEqual({ + customerId: expect.objectContaining({ value: 'customer-1' }), + environment: expect.objectContaining({ value: 'prod' }), + }); + }); + + it('does not merge distinct composite tuples that share separator-like characters', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['customerId', 'environment'], + conflictingProperties: [ + { fullPath: 'customerId', column: 'customerId' }, + { fullPath: 'environment', column: 'environment' }, + ], + }, + ]; + + const where = buildWhereConditions( + [ + { customerId: 'a:b', environment: 'c' }, + { customerId: 'a', environment: 'b:c' }, + ], + groups, + ); + + expect(where).toHaveLength(2); + }); + + it('ORs single-column and composite groups together', () => { + const groups: ConflictingFieldGroup[] = [ + { + baseFields: ['id'], + conflictingProperties: [{ fullPath: 'id', column: 'id' }], + }, + { + baseFields: ['customerId', 'environment'], + conflictingProperties: [ + { fullPath: 'customerId', column: 'customerId' }, + { fullPath: 'environment', column: 'environment' }, + ], + }, + ]; + + const where = buildWhereConditions( + [{ id: 'record-1', customerId: 'customer-1', environment: 'prod' }], + groups, + ); + + expect(where).toHaveLength(2); + + const idCondition = where.find((condition) => 'id' in condition); + const compositeCondition = where.find( + (condition) => 'customerId' in condition, + ); + + expect(idCondition?.id.type.toLowerCase()).toBe('in'); + expect(idCondition?.id.value).toEqual(['record-1']); + expect(compositeCondition?.customerId.type.toLowerCase()).toBe('equal'); + expect(compositeCondition?.environment.type.toLowerCase()).toBe('equal'); }); }); diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/build-where-conditions.util.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/build-where-conditions.util.ts index 4ffcab0ee6..4652809ae4 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/build-where-conditions.util.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/build-where-conditions.util.ts @@ -1,27 +1,115 @@ import { type ObjectRecord } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; -import { type FindOperator, In } from 'typeorm'; +import { Equal, In, type FindOperator } from 'typeorm'; -import { type ConflictingFieldGroup } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type'; +import { + type ConflictingFieldGroup, + type ConflictingFieldValue, + type ConflictingProperty, +} from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type'; import { getValueFromPath } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-value-from-path.util'; +type WhereCondition = Record>; + +const buildCompositeConditionKey = ( + conditionEntries: [string, ConflictingFieldValue][], +): string => { + const sortedEntries = [...conditionEntries].sort(([columnA], [columnB]) => + columnA.localeCompare(columnB), + ); + + return JSON.stringify(sortedEntries); +}; + +const buildSingleColumnCondition = ( + records: Partial[], + conflictingProperty: ConflictingProperty, +): WhereCondition | undefined => { + const distinctValues = [ + ...new Set( + records + .map((record) => getValueFromPath(record, conflictingProperty.fullPath)) + .filter(isDefined), + ), + ]; + + if (distinctValues.length === 0) { + return undefined; + } + + return { [conflictingProperty.column]: In(distinctValues) }; +}; + +const buildCompositeConditionEntries = ( + record: Partial, + conflictingProperties: ConflictingProperty[], +): [string, ConflictingFieldValue][] | undefined => { + const conditionEntries: [string, ConflictingFieldValue][] = []; + + for (const conflictingProperty of conflictingProperties) { + const fieldValue = getValueFromPath(record, conflictingProperty.fullPath); + + if (!isDefined(fieldValue)) { + return undefined; + } + + conditionEntries.push([conflictingProperty.column, fieldValue]); + } + + return conditionEntries; +}; + export const buildWhereConditions = ( records: Partial[], conflictingFieldGroups: ConflictingFieldGroup[], -): Record>[] => { - const whereConditions: Record>[] = []; +): WhereCondition[] => { + const whereConditions: WhereCondition[] = []; + const seenCompositeConditionKeys = new Set(); - for (const conflictingProperty of conflictingFieldGroups.flatMap( - (group) => group.conflictingProperties, - )) { - const fieldValues = records - .map((record) => getValueFromPath(record, conflictingProperty.fullPath)) - .filter(isDefined); + for (const conflictingFieldGroup of conflictingFieldGroups) { + const { conflictingProperties } = conflictingFieldGroup; - if (fieldValues.length > 0) { - whereConditions.push({ - [conflictingProperty.column]: In(fieldValues), - }); + if (conflictingProperties.length === 1) { + const condition = buildSingleColumnCondition( + records, + conflictingProperties[0], + ); + + if (isDefined(condition)) { + whereConditions.push(condition); + } + + continue; + } + + for (const record of records) { + const conditionEntries = buildCompositeConditionEntries( + record, + conflictingProperties, + ); + + if (!isDefined(conditionEntries)) { + continue; + } + + const conditionKey = buildCompositeConditionKey(conditionEntries); + + if (seenCompositeConditionKeys.has(conditionKey)) { + continue; + } + + seenCompositeConditionKeys.add(conditionKey); + + whereConditions.push( + conditionEntries.reduce( + (accumulator, [column, value]) => { + accumulator[column] = Equal(value); + + return accumulator; + }, + {}, + ), + ); } } diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-value-from-path.util.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-value-from-path.util.ts index 35bd1db396..411dc35fb6 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-value-from-path.util.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-value-from-path.util.ts @@ -1,9 +1,11 @@ import { type ObjectRecord } from 'twenty-shared/types'; +import { type ConflictingFieldValue } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type'; + export const getValueFromPath = ( record: Partial, path: string, -): string | undefined => { +): ConflictingFieldValue | undefined => { const pathParts = path.split('.'); if (pathParts.length === 1) {