Fix duplicate searchFieldMetadata inserts in the 2.16 backfill upgrade command (#23060)

## Context A self-hosted instance upgrading from 2.0.3 to v2.22.0 got stuck with one workspace failing at `2.16.0_BackfillSearchFieldMetadataCommand`: ``` [QueryFailedError] duplicate key value violates unique constraint "IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE" Detail: Key ("objectMetadataId", "fieldMetadataId")=(...) already exists. ``` The failure happened on a retry after a previous partial run, and reproduced even though the command already recomputes `flatSearchFieldMetadataMaps` before deriving the create-set (#22884). ## Root cause The idempotency dedupe compares `(objectMetadataId, fieldMetadataId)` pairs across two differently-fresh caches: - The **existing rows** side comes from `flatSearchFieldMetadataMaps`, which is recomputed from the database (real current ids). - The **candidate** side resolves ids through `flatObjectMetadataMaps` / `flatFieldMetadataMaps`, which are **not** invalidated. During a cross-version upgrade these can be stale, since the migration runner only invalidates the cache keys a migration touched. When a stale map resolves a candidate to an outdated id, the dedupe key doesn't match the existing row and the row is re-emitted. The migration runner then re-resolves the universal identifiers against fresh maps at execution time and inserts with the real current ids — exactly the pair already committed by the earlier partial run (each per-application migration commits independently) — tripping the unique constraint and failing the upgrade. ## Fix Two independent layers, either of which would have prevented the failure: 1. **Consistent snapshot for the build phase**: the command now invalidates and recomputes all three maps the dedupe depends on (`flatObjectMetadataMaps`, `flatFieldMetadataMaps`, `flatSearchFieldMetadataMaps`), so candidate resolution, existing-row keys, and the runner all see the same database state. 2. **Id-churn-proof dedupe**: every row this command creates carries a deterministic universal identifier (`getSearchFieldUniversalIdentifier`, derived from application + field universal identifiers, no database ids involved) and `(workspaceId, universalIdentifier)` is unique. The build util now also skips any candidate whose deterministic universal identifier already exists, catching leftovers from a previous partial run even if objects/fields were recreated under new ids in between. Deliberately **not** done: `ON CONFLICT DO NOTHING` in the create action handler — it is shared by all runtime `searchFieldMetadata` creation, and swallowing a conflict would leave the flat-entity cache holding an entity id that differs from the row actually in the database. ## Test Added a regression test reproducing the failure shape: an existing row with the same deterministic universal identifier but stale metadata ids must not be re-emitted by the backfill. Note: `ReconcileSearchFieldMetadataCommand` (2.20) has the same stale-cache exposure; hardening it is left to a follow-up.  <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23060?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-20 15:23:26 +02:00
parent 98c71d7b3d
commit 6b55a6b51c
3 changed files with 82 additions and 10 deletions
@@ -34,11 +34,16 @@ export class BackfillSearchFieldMetadataCommand extends ProvisionedWorkspaceComm
    const isDryRun = options.dryRun ?? false;

    // The migration runner only invalidates the flat-maps keys a migration touched,
-    // so during a cross-version upgrade earlier commands can leave this map stale.
-    // A stale map breaks the existing-rows dedupe below and re-inserts rows,
-    // tripping IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE. Recompute from the
-    // database before deriving the create-set.
+    // so during a cross-version upgrade earlier commands can leave these maps stale.
+    // The existing-rows dedupe below compares (objectMetadataId, fieldMetadataId)
+    // pairs across maps: candidate ids resolved from a stale object/field map won't
+    // match the fresh search map's ids, so already-created rows are re-emitted and
+    // the runner (which re-resolves universal identifiers against fresh maps) trips
+    // IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE. Recompute every map the dedupe
+    // depends on from the database before deriving the create-set.
    await this.workspaceCacheService.invalidateAndRecompute(workspaceId, [
+      'flatObjectMetadataMaps',
+      'flatFieldMetadataMaps',
      'flatSearchFieldMetadataMaps',
    ]);

@@ -1,4 +1,7 @@
-import { TWENTY_STANDARD_APPLICATION_UNIVERSAL_IDENTIFIER } from 'twenty-shared/application';
+import {
+  getSearchFieldUniversalIdentifier,
+  TWENTY_STANDARD_APPLICATION_UNIVERSAL_IDENTIFIER,
+} from 'twenty-shared/application';
 import { FieldMetadataType } from 'twenty-shared/types';

 import { buildSearchFieldMetadataBackfillOperations } from 'src/database/commands/upgrade-version-command/2-16/utils/build-search-field-metadata-backfill-operations.util';
@@ -622,6 +625,54 @@ describe('buildSearchFieldMetadataBackfillOperations', () => {
    ).toHaveLength(0);
  });

+  it('skips a row whose deterministic universal identifier already exists even when the existing row carries stale metadata ids', () => {
+    const { customObject, nameField, nameDescriptionField, searchVectorField } =
+      buildCustomObjectFixture();
+
+    // Simulates a retry after a partial run during a cross-version upgrade: the
+    // previously committed row still points at the ids the metadata had at insert
+    // time, while the object/field maps now expose new ids (id churn from earlier
+    // upgrade commands). The (objectMetadataId, fieldMetadataId) dedupe misses the
+    // pair, but the deterministic universal identifier is unchanged and must
+    // prevent re-emitting the row.
+    const existingSearchFieldMetadata = buildSearchFieldMetadata({
+      id: 'existing-search-field-id',
+      universalIdentifier: getSearchFieldUniversalIdentifier({
+        applicationUniversalIdentifier:
+          CUSTOM_APPLICATION_UNIVERSAL_IDENTIFIER,
+        fieldMetadataUniversalIdentifier: nameField.universalIdentifier,
+      }),
+      objectMetadataId: 'stale-object-metadata-id',
+      fieldMetadataId: 'stale-field-metadata-id',
+      objectMetadataUniversalIdentifier: customObject.universalIdentifier,
+      fieldMetadataUniversalIdentifier: nameField.universalIdentifier,
+      applicationUniversalIdentifier: CUSTOM_APPLICATION_UNIVERSAL_IDENTIFIER,
+    });
+
+    const { flatSearchFieldMetadatasToCreateByApplicationUniversalIdentifier } =
+      buildSearchFieldMetadataBackfillOperations({
+        flatObjectMetadataMaps: buildFlatObjectMetadataMaps([customObject]),
+        flatFieldMetadataMaps: buildFlatFieldMetadataMaps([
+          nameField,
+          nameDescriptionField,
+          searchVectorField,
+        ]),
+        flatSearchFieldMetadataMaps: buildFlatSearchFieldMetadataMaps([
+          existingSearchFieldMetadata,
+        ]),
+        standardFlatSearchFieldMetadataMaps: buildFlatSearchFieldMetadataMaps(
+          [],
+        ),
+        customApplicationId: CUSTOM_APPLICATION_ID,
+      });
+
+    expect(
+      Object.keys(
+        flatSearchFieldMetadatasToCreateByApplicationUniversalIdentifier,
+      ),
+    ).toHaveLength(0);
+  });
+
  it('skips a custom object whose name field is not a searchable type', () => {
    const customObjectId = 'relation-name-object-id';
    const customObjectUniversalIdentifier = 'relation-name-object-uid';
@@ -94,16 +94,32 @@ export const buildSearchFieldMetadataBackfillOperations = ({
      return;
    }

-    candidateSearchFieldMetadataKeys.add(searchFieldMetadataKey);
-
-    flatSearchFieldMetadatasToCreate.push(
-      buildFlatSearchFieldMetadataForField({
+    const universalFlatSearchFieldMetadata = buildFlatSearchFieldMetadataForField(
+      {
        flatObjectMetadata,
        flatFieldMetadata,
        tsVectorFlatFieldMetadata,
        position,
-      }),
+      },
    );
+
+    // Second dedupe layer, immune to metadata-id churn: rows created by a previous
+    // partial run of this command carry the same deterministic universal identifier
+    // (unique per workspace). The id-based check above can miss them when earlier
+    // upgrade commands recreated objects/fields under new ids.
+    if (
+      isDefined(
+        flatSearchFieldMetadataMaps.byUniversalIdentifier[
+          universalFlatSearchFieldMetadata.universalIdentifier
+        ],
+      )
+    ) {
+      return;
+    }
+
+    candidateSearchFieldMetadataKeys.add(searchFieldMetadataKey);
+
+    flatSearchFieldMetadatasToCreate.push(universalFlatSearchFieldMetadata);
  };

  // Standard objects: mirror exactly what provisioning/standard-sync creates.