perf(upsert) - tighten createMany upsert candidate lookup to avoid broad OR scans (#22721)

## 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.  <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22721?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
2026-07-09 15:47:54 +02:00
parent b2c2c2341c
commit 7475b5f16f
4 changed files with 296 additions and 34 deletions
@@ -1,3 +1,5 @@
+export type ConflictingFieldValue = string | number | boolean;
+
 export type ConflictingProperty = {
  fullPath: string;
  column: string;
@@ -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');
  });
 });
@@ -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<string, FindOperator<ConflictingFieldValue>>;
+
+const buildCompositeConditionKey = (
+  conditionEntries: [string, ConflictingFieldValue][],
+): string => {
+  const sortedEntries = [...conditionEntries].sort(([columnA], [columnB]) =>
+    columnA.localeCompare(columnB),
+  );
+
+  return JSON.stringify(sortedEntries);
+};
+
+const buildSingleColumnCondition = (
+  records: Partial<ObjectRecord>[],
+  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<ObjectRecord>,
+  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<ObjectRecord>[],
  conflictingFieldGroups: ConflictingFieldGroup[],
-): Record<string, FindOperator<string>>[] => {
-  const whereConditions: Record<string, FindOperator<string>>[] = [];
+): WhereCondition[] => {
+  const whereConditions: WhereCondition[] = [];
+  const seenCompositeConditionKeys = new Set<string>();

-  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<WhereCondition>(
+          (accumulator, [column, value]) => {
+            accumulator[column] = Equal(value);
+
+            return accumulator;
+          },
+          {},
+        ),
+      );
    }
  }

@@ -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<ObjectRecord>,
  path: string,
-): string | undefined => {
+): ConflictingFieldValue | undefined => {
  const pathParts = path.split('.');

  if (pathParts.length === 1) {