From 0db4ddd46e0c928bcd38ca7b9d7091d9e5e2ba40 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:00:48 +0200 Subject: [PATCH] fix(server): tolerate metadata-physical index drift in legacy index name normalization (#22472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context The 2.18 `NormalizeLegacyIndexNames` workspace command (`1799200000000`, introduced in #22053) fails several production workspaces with `42P01 relation … does not exist`, rolling back the whole per-workspace upgrade transaction and marking the workspace `Failed`. ## Root cause The command assumes the physical index in the workspace schema is named exactly as recorded in `core."indexMetadata"."name"`. For workspaces where the physical index was already rebuilt/renamed under the v2 deterministic name (only targeted phone/relation rebuilds got new names after #14567) while metadata kept the legacy hash, the rename source no longer exists, so `ALTER INDEX … RENAME` aborts the entire workspace upgrade. (The duplicate-drop path uses `DROP INDEX IF EXISTS` and is unaffected.) ## Fix - **`WorkspaceSchemaIndexManagerService`**: new `doesIndexExist` and `getIndexDefinition` helpers querying `pg_indexes` for a `(schema, index)` pair. `renameIndexWithoutRebuild` keeps its strict semantics (no `IF EXISTS`) — drift tolerance lives in the command, which is the only caller that expects it. - **`NormalizeLegacyIndexNamesCommand`** — the rename operation now reconciles drift instead of blindly renaming: - Target name already exists physically, source gone → skip the rename, just point `indexMetadata.name` at it (the common "physical already v2, metadata still legacy" case). - Both source and target exist physically → compare their `pg_indexes.indexdef` ignoring the name: if identical, drop the legacy duplicate (it would otherwise be orphaned forever since metadata stops referencing it, adding permanent write/maintenance cost); if the definitions differ, keep it in place and log a warning. - Source exists, target free → rename as before, then update metadata. - Neither exists → log a warning and update metadata so a future rebuild recreates the index under the expected v2 name. In every branch the metadata name ends up on the recomputed v2 name, and no missing physical index can abort the workspace transaction anymore. ## Tests - Regression tests on the command spec for the four drift cases (target-already-renamed, both-missing, both-present-identical → drop, both-present-different → keep); existing rename/duplicate/dry-run/rollback tests updated to declare the physical indexes present. - New spec for `WorkspaceSchemaIndexManagerService` covering the rename SQL, the `pg_indexes` existence check, and the definition lookup. - New spec for `areIndexDefinitionsEquivalent` (name-only diff, uniqueness, columns, where clause, malformed input). `npx jest` on all three specs (20 passed), `lint:diff-with-main` and `typecheck` green. --- ...00-normalize-legacy-index-names.command.ts | 342 +++++++++++++----- ...rmalize-legacy-index-names.command.spec.ts | 256 ++++++++++++- ...-index-definitions-equivalent.util.spec.ts | 67 ++++ .../are-index-definitions-equivalent.util.ts | 28 ++ .../utils/does-physical-index-exist.util.ts | 34 ++ .../get-physical-index-definition.util.ts | 32 ++ 6 files changed, 668 insertions(+), 91 deletions(-) create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/__tests__/are-index-definitions-equivalent.util.spec.ts create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/are-index-definitions-equivalent.util.ts create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util.ts create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util.ts diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.ts index 010c626f3e..5441fdcaa5 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.ts @@ -1,15 +1,23 @@ import { Command } from 'nest-commander'; import { isDefined } from 'twenty-shared/utils'; +import { type QueryRunner } from 'typeorm'; import { ActiveOrSuspendedWorkspaceCommandRunner } from 'src/database/commands/command-runners/active-or-suspended-workspace.command-runner'; import { WorkspaceIteratorService } from 'src/database/commands/command-runners/workspace-iterator.service'; import { type RunOnWorkspaceArgs } from 'src/database/commands/command-runners/workspace.command-runner'; +import { areIndexDefinitionsEquivalent } from 'src/database/commands/upgrade-version-command/2-18/utils/are-index-definitions-equivalent.util'; +import { doesPhysicalIndexExist } from 'src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util'; +import { getPhysicalIndexDefinition } from 'src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util'; import { type FlatIndexNameStatus, + type IndexNameNormalizationOperation, planIndexNameNormalization, + type RenameIndexNameOperation, } from 'src/database/commands/upgrade-version-command/2-18/utils/plan-index-name-normalization.util'; import { RegisteredWorkspaceCommand } from 'src/engine/core-modules/upgrade/decorators/registered-workspace-command.decorator'; import { findManyFlatEntityByUniversalIdentifierInUniversalFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-many-flat-entity-by-universal-identifier-in-universal-flat-entity-maps-or-throw.util'; +import { getMetadataFlatEntityMapsKey } from 'src/engine/metadata-modules/flat-entity/utils/get-metadata-flat-entity-maps-key.util'; +import { getMetadataRelatedMetadataNames } from 'src/engine/metadata-modules/flat-entity/utils/get-metadata-related-metadata-names.util'; import { generateFlatIndexMetadataWithNameOrThrow } from 'src/engine/metadata-modules/index-metadata/utils/generate-flat-index.util'; import { WorkspaceSchemaManagerService } from 'src/engine/twenty-orm/workspace-schema-manager/workspace-schema-manager.service'; import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service'; @@ -45,6 +53,98 @@ export class NormalizeLegacyIndexNamesCommand extends ActiveOrSuspendedWorkspace return; } + const operations = + await this.computeIndexNameNormalizationOperations(workspaceId); + + if (operations.length === 0) { + this.logger.log( + `No legacy index names to normalize for workspace ${workspaceId}, skipping`, + ); + + return; + } + + if (options.dryRun) { + for (const operation of operations) { + if (operation.type === 'rename') { + this.logger.log( + `[DRY RUN] Would rename index ${operation.fromName} -> ${operation.toName} (workspace ${workspaceId})`, + ); + } else { + this.logger.log( + `[DRY RUN] Would drop redundant duplicate index ${operation.redundantName} (kept ${operation.keptName}) (workspace ${workspaceId})`, + ); + } + } + + return; + } + + const schemaName = getWorkspaceSchemaName(workspaceId); + const queryRunner = dataSource.createQueryRunner(); + let isQueryRunnerConnected = false; + let isTransactionStarted = false; + + try { + await queryRunner.connect(); + isQueryRunnerConnected = true; + + await queryRunner.startTransaction(); + isTransactionStarted = true; + + for (const [operationIndex, operation] of operations.entries()) { + const savepointName = `index_name_normalization_${operationIndex}`; + + await queryRunner.query(`SAVEPOINT "${savepointName}"`); + + try { + await this.applyIndexNameNormalizationOperation({ + queryRunner, + schemaName, + operation, + workspaceId, + }); + + await queryRunner.query(`RELEASE SAVEPOINT "${savepointName}"`); + } catch (error) { + await queryRunner.query(`ROLLBACK TO SAVEPOINT "${savepointName}"`); + + this.logger.warn( + `Skipping index name normalization for ${operation.type === 'rename' ? operation.fromName : operation.redundantName} in workspace ${workspaceId}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + + await queryRunner.commitTransaction(); + } catch (error) { + if (isTransactionStarted) { + await queryRunner.rollbackTransaction(); + } + + throw error; + } finally { + if (isQueryRunnerConnected) { + await queryRunner.release(); + } + } + + const indexRelatedFlatMapsKeys = [ + ...new Set( + ['index' as const, ...getMetadataRelatedMetadataNames('index')].map( + getMetadataFlatEntityMapsKey, + ), + ), + ]; + + await this.workspaceCacheService.invalidateAndRecompute( + workspaceId, + indexRelatedFlatMapsKeys, + ); + } + + private async computeIndexNameNormalizationOperations( + workspaceId: string, + ): Promise { const { flatIndexMaps, flatObjectMetadataMaps, flatFieldMetadataMaps } = await this.workspaceCacheService.getOrRecompute(workspaceId, [ 'flatIndexMaps', @@ -104,103 +204,173 @@ export class NormalizeLegacyIndexNamesCommand extends ActiveOrSuspendedWorkspace } } - const operations = planIndexNameNormalization(indexStatuses); + return planIndexNameNormalization(indexStatuses); + } + + private async applyIndexNameNormalizationOperation({ + queryRunner, + schemaName, + operation, + workspaceId, + }: { + queryRunner: QueryRunner; + schemaName: string; + operation: IndexNameNormalizationOperation; + workspaceId: string; + }): Promise { + if (operation.type === 'rename') { + const targetIndexExists = await doesPhysicalIndexExist({ + queryRunner, + schemaName, + indexName: operation.toName, + }); + + if (targetIndexExists) { + await this.reconcileRenameWithExistingTargetIndex({ + queryRunner, + schemaName, + operation, + workspaceId, + }); + } else { + await this.renameIndexToExpectedName({ + queryRunner, + schemaName, + operation, + workspaceId, + }); + } + + await queryRunner.query( + `UPDATE "core"."indexMetadata" + SET "name" = $1 + WHERE "id" = $2 + AND "workspaceId" = $3`, + [operation.toName, operation.indexMetadataId, workspaceId], + ); + } else { + await dropIndexFromWorkspaceSchema({ + indexName: operation.redundantName, + workspaceSchemaManagerService: this.workspaceSchemaManagerService, + queryRunner, + schemaName, + }); + + await deleteIndexMetadata({ + entityId: operation.indexMetadataId, + queryRunner, + workspaceId, + }); - if (operations.length === 0) { this.logger.log( - `No legacy index names to normalize for workspace ${workspaceId}, skipping`, + `Dropped redundant duplicate index ${operation.redundantName} (kept ${operation.keptName}) (workspace ${workspaceId})`, + ); + } + } + + // Target index already exists physically: the legacy index cannot be + // renamed onto it. Metadata stops referencing the legacy index, so nothing + // would ever clean it up: drop it when it is a true duplicate of the + // target, keep it only when the definitions differ. + private async reconcileRenameWithExistingTargetIndex({ + queryRunner, + schemaName, + operation, + workspaceId, + }: { + queryRunner: QueryRunner; + schemaName: string; + operation: RenameIndexNameOperation; + workspaceId: string; + }): Promise { + const sourceIndexExists = await doesPhysicalIndexExist({ + queryRunner, + schemaName, + indexName: operation.fromName, + }); + + if (!sourceIndexExists) { + this.logger.log( + `Index already physically named ${operation.toName}, reconciling metadata only (was ${operation.fromName}) (workspace ${workspaceId})`, ); return; } - if (options.dryRun) { - for (const operation of operations) { - if (operation.type === 'rename') { - this.logger.log( - `[DRY RUN] Would rename index ${operation.fromName} -> ${operation.toName} (workspace ${workspaceId})`, - ); - } else { - this.logger.log( - `[DRY RUN] Would drop redundant duplicate index ${operation.redundantName} (kept ${operation.keptName}) (workspace ${workspaceId})`, - ); - } - } + const sourceIndexDefinition = await getPhysicalIndexDefinition({ + queryRunner, + schemaName, + indexName: operation.fromName, + }); + const targetIndexDefinition = await getPhysicalIndexDefinition({ + queryRunner, + schemaName, + indexName: operation.toName, + }); + + if ( + isDefined(sourceIndexDefinition) && + isDefined(targetIndexDefinition) && + areIndexDefinitionsEquivalent({ + indexDefinitionA: sourceIndexDefinition, + indexDefinitionB: targetIndexDefinition, + }) + ) { + await this.workspaceSchemaManagerService.indexManager.dropIndex({ + queryRunner, + schemaName, + indexName: operation.fromName, + }); + + this.logger.log( + `Dropped duplicate legacy index ${operation.fromName} (identical definition already exists as ${operation.toName}) (workspace ${workspaceId})`, + ); + } else { + this.logger.warn( + `Index ${operation.toName} already exists physically alongside ${operation.fromName} with a different definition; leaving ${operation.fromName} in place and pointing metadata at ${operation.toName} (workspace ${workspaceId})`, + ); + } + } + + // Target index does not exist physically: rename the legacy index in place + // when present, otherwise only the metadata name will be updated so a + // future rebuild uses the expected name. + private async renameIndexToExpectedName({ + queryRunner, + schemaName, + operation, + workspaceId, + }: { + queryRunner: QueryRunner; + schemaName: string; + operation: RenameIndexNameOperation; + workspaceId: string; + }): Promise { + const sourceIndexExists = await doesPhysicalIndexExist({ + queryRunner, + schemaName, + indexName: operation.fromName, + }); + + if (!sourceIndexExists) { + this.logger.warn( + `Neither ${operation.fromName} nor ${operation.toName} exists physically; updating metadata name so a future rebuild uses the expected name (workspace ${workspaceId})`, + ); return; } - const schemaName = getWorkspaceSchemaName(workspaceId); - const queryRunner = dataSource.createQueryRunner(); - let isQueryRunnerConnected = false; - let isTransactionStarted = false; + await this.workspaceSchemaManagerService.indexManager.renameIndexWithoutRebuild( + { + queryRunner, + schemaName, + fromIndexName: operation.fromName, + toIndexName: operation.toName, + }, + ); - try { - await queryRunner.connect(); - isQueryRunnerConnected = true; - - await queryRunner.startTransaction(); - isTransactionStarted = true; - - for (const operation of operations) { - if (operation.type === 'rename') { - await this.workspaceSchemaManagerService.indexManager.renameIndexWithoutRebuild( - { - queryRunner, - schemaName, - fromIndexName: operation.fromName, - toIndexName: operation.toName, - }, - ); - - await queryRunner.query( - `UPDATE "core"."indexMetadata" - SET "name" = $1 - WHERE "id" = $2 - AND "workspaceId" = $3`, - [operation.toName, operation.indexMetadataId, workspaceId], - ); - - this.logger.log( - `Renamed index ${operation.fromName} -> ${operation.toName} (workspace ${workspaceId})`, - ); - } else { - await dropIndexFromWorkspaceSchema({ - indexName: operation.redundantName, - workspaceSchemaManagerService: this.workspaceSchemaManagerService, - queryRunner, - schemaName, - }); - - await deleteIndexMetadata({ - entityId: operation.indexMetadataId, - queryRunner, - workspaceId, - }); - - this.logger.log( - `Dropped redundant duplicate index ${operation.redundantName} (kept ${operation.keptName}) (workspace ${workspaceId})`, - ); - } - } - - await queryRunner.commitTransaction(); - } catch (error) { - if (isTransactionStarted) { - await queryRunner.rollbackTransaction(); - } - - throw error; - } finally { - if (isQueryRunnerConnected) { - await queryRunner.release(); - } - } - - await this.workspaceCacheService.invalidateAndRecompute(workspaceId, [ - 'flatIndexMaps', - 'flatObjectMetadataMaps', - 'flatFieldMetadataMaps', - ]); + this.logger.log( + `Renamed index ${operation.fromName} -> ${operation.toName} (workspace ${workspaceId})`, + ); } } diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/__tests__/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.spec.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/__tests__/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.spec.ts index cbbc207a0e..5c2ee8bd18 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/__tests__/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.spec.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/__tests__/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command.spec.ts @@ -1,6 +1,8 @@ import { type DataSource, type QueryRunner } from 'typeorm'; import { NormalizeLegacyIndexNamesCommand } from 'src/database/commands/upgrade-version-command/2-18/2-18-workspace-command-1799200000000-normalize-legacy-index-names.command'; +import { doesPhysicalIndexExist } from 'src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util'; +import { getPhysicalIndexDefinition } from 'src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util'; import { type WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service'; import { type WorkspaceIteratorService } from 'src/database/commands/command-runners/workspace-iterator.service'; import { type WorkspaceSchemaManagerService } from 'src/engine/twenty-orm/workspace-schema-manager/workspace-schema-manager.service'; @@ -16,11 +18,19 @@ jest.mock( jest.mock( 'src/engine/workspace-manager/workspace-migration/workspace-migration-runner/action-handlers/index/utils/index-action-handler.utils', ); +jest.mock( + 'src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util', +); +jest.mock( + 'src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util', +); const generateFlatIndexMetadataWithNameOrThrowMock = generateFlatIndexMetadataWithNameOrThrow as jest.Mock; const deleteIndexMetadataMock = deleteIndexMetadata as jest.Mock; const dropIndexFromWorkspaceSchemaMock = dropIndexFromWorkspaceSchema as jest.Mock; +const doesPhysicalIndexExistMock = doesPhysicalIndexExist as jest.Mock; +const getPhysicalIndexDefinitionMock = getPhysicalIndexDefinition as jest.Mock; const WORKSPACE_ID = '20202020-0000-0000-0000-000000000001'; @@ -103,6 +113,22 @@ describe('NormalizeLegacyIndexNamesCommand', () => { let getOrRecomputeMock: jest.Mock; let invalidateAndRecomputeMock: jest.Mock; + const setPhysicalIndexes = (physicalIndexNames: string[]) => { + doesPhysicalIndexExistMock.mockImplementation( + ({ indexName }: { indexName: string }) => + Promise.resolve(physicalIndexNames.includes(indexName)), + ); + }; + + const setPhysicalIndexDefinitions = ( + definitionsByName: Record, + ) => { + getPhysicalIndexDefinitionMock.mockImplementation( + ({ indexName }: { indexName: string }) => + Promise.resolve(definitionsByName[indexName] ?? null), + ); + }; + beforeEach(() => { jest.clearAllMocks(); @@ -151,6 +177,8 @@ describe('NormalizeLegacyIndexNamesCommand', () => { ]), ); + setPhysicalIndexes(['legacyhash']); + const { queryRunner, query, commit } = buildQueryRunner(); const dataSource = { createQueryRunner: () => queryRunner, @@ -174,7 +202,6 @@ describe('NormalizeLegacyIndexNamesCommand', () => { expect(invalidateAndRecomputeMock).toHaveBeenCalledWith(WORKSPACE_ID, [ 'flatIndexMaps', 'flatObjectMetadataMaps', - 'flatFieldMetadataMaps', ]); }); @@ -196,6 +223,8 @@ describe('NormalizeLegacyIndexNamesCommand', () => { ]), ); + setPhysicalIndexes(['legacyA', 'legacyB']); + const { queryRunner } = buildQueryRunner(); const dataSource = { createQueryRunner: () => queryRunner, @@ -264,7 +293,7 @@ describe('NormalizeLegacyIndexNamesCommand', () => { expect(invalidateAndRecomputeMock).not.toHaveBeenCalled(); }); - it('rolls back the transaction when an operation fails', async () => { + it('reconciles metadata without renaming when the physical index already carries the expected name', async () => { getOrRecomputeMock.mockResolvedValue( buildFlatEntityMaps([ { @@ -276,14 +305,231 @@ describe('NormalizeLegacyIndexNamesCommand', () => { ]), ); - renameIndexMock.mockRejectedValueOnce(new Error('boom')); + // Metadata drift: the physical index was already renamed to the v2 name. + setPhysicalIndexes(['IDX_UNIQUE_new']); - const { queryRunner, commit, rollback } = buildQueryRunner(); + const { queryRunner, query, commit } = buildQueryRunner(); const dataSource = { createQueryRunner: () => queryRunner, } as unknown as DataSource; - await expect(runOnWorkspace(dataSource)).rejects.toThrow('boom'); + await runOnWorkspace(dataSource); + + expect(renameIndexMock).not.toHaveBeenCalled(); + expect(query).toHaveBeenCalledWith(expect.stringContaining('UPDATE'), [ + 'IDX_UNIQUE_new', + 'index-1', + WORKSPACE_ID, + ]); + expect(commit).toHaveBeenCalled(); + expect(invalidateAndRecomputeMock).toHaveBeenCalledWith(WORKSPACE_ID, [ + 'flatIndexMaps', + 'flatObjectMetadataMaps', + ]); + }); + + it('skips the physical rename but still reconciles metadata when neither source nor target index exists', async () => { + getOrRecomputeMock.mockResolvedValue( + buildFlatEntityMaps([ + { + universalIdentifier: 'idx-uid', + id: 'index-1', + name: 'legacyhash', + expectedName: 'IDX_UNIQUE_new', + }, + ]), + ); + + setPhysicalIndexes([]); + + const { queryRunner, query, commit } = buildQueryRunner(); + const dataSource = { + createQueryRunner: () => queryRunner, + } as unknown as DataSource; + + await runOnWorkspace(dataSource); + + expect(renameIndexMock).not.toHaveBeenCalled(); + expect(query).toHaveBeenCalledWith(expect.stringContaining('UPDATE'), [ + 'IDX_UNIQUE_new', + 'index-1', + WORKSPACE_ID, + ]); + expect(commit).toHaveBeenCalled(); + expect(invalidateAndRecomputeMock).toHaveBeenCalledWith(WORKSPACE_ID, [ + 'flatIndexMaps', + 'flatObjectMetadataMaps', + ]); + }); + + it('drops the duplicate legacy index when both source and target exist with the same definition', async () => { + getOrRecomputeMock.mockResolvedValue( + buildFlatEntityMaps([ + { + universalIdentifier: 'idx-uid', + id: 'index-1', + name: 'legacyhash', + expectedName: 'IDX_UNIQUE_new', + }, + ]), + ); + + setPhysicalIndexes(['legacyhash', 'IDX_UNIQUE_new']); + setPhysicalIndexDefinitions({ + legacyhash: + 'CREATE UNIQUE INDEX legacyhash ON workspace_test."myObject" USING btree (name)', + IDX_UNIQUE_new: + 'CREATE UNIQUE INDEX "IDX_UNIQUE_new" ON workspace_test."myObject" USING btree (name)', + }); + + const { queryRunner, query, commit } = buildQueryRunner(); + const dataSource = { + createQueryRunner: () => queryRunner, + } as unknown as DataSource; + + await runOnWorkspace(dataSource); + + expect(renameIndexMock).not.toHaveBeenCalled(); + expect(dropIndexMock).toHaveBeenCalledWith( + expect.objectContaining({ indexName: 'legacyhash' }), + ); + expect(query).toHaveBeenCalledWith(expect.stringContaining('UPDATE'), [ + 'IDX_UNIQUE_new', + 'index-1', + WORKSPACE_ID, + ]); + expect(commit).toHaveBeenCalled(); + expect(invalidateAndRecomputeMock).toHaveBeenCalledWith(WORKSPACE_ID, [ + 'flatIndexMaps', + 'flatObjectMetadataMaps', + ]); + }); + + it('leaves the orphan source index in place when both exist but with different definitions', async () => { + getOrRecomputeMock.mockResolvedValue( + buildFlatEntityMaps([ + { + universalIdentifier: 'idx-uid', + id: 'index-1', + name: 'legacyhash', + expectedName: 'IDX_UNIQUE_new', + }, + ]), + ); + + setPhysicalIndexes(['legacyhash', 'IDX_UNIQUE_new']); + setPhysicalIndexDefinitions({ + legacyhash: + 'CREATE INDEX legacyhash ON workspace_test."myObject" USING btree ("createdAt")', + IDX_UNIQUE_new: + 'CREATE UNIQUE INDEX "IDX_UNIQUE_new" ON workspace_test."myObject" USING btree (name)', + }); + + const { queryRunner, query, commit } = buildQueryRunner(); + const dataSource = { + createQueryRunner: () => queryRunner, + } as unknown as DataSource; + + await runOnWorkspace(dataSource); + + expect(renameIndexMock).not.toHaveBeenCalled(); + expect(dropIndexMock).not.toHaveBeenCalled(); + expect(dropIndexFromWorkspaceSchemaMock).not.toHaveBeenCalled(); + expect(query).toHaveBeenCalledWith(expect.stringContaining('UPDATE'), [ + 'IDX_UNIQUE_new', + 'index-1', + WORKSPACE_ID, + ]); + expect(commit).toHaveBeenCalled(); + expect(invalidateAndRecomputeMock).toHaveBeenCalledWith(WORKSPACE_ID, [ + 'flatIndexMaps', + 'flatObjectMetadataMaps', + ]); + }); + + it('rolls back to the savepoint and continues with remaining operations when one fails', async () => { + getOrRecomputeMock.mockResolvedValue( + buildFlatEntityMaps([ + { + universalIdentifier: 'idx-a', + id: 'index-a', + name: 'legacyA', + expectedName: 'IDX_UNIQUE_newA', + }, + { + universalIdentifier: 'idx-b', + id: 'index-b', + name: 'legacyB', + expectedName: 'IDX_UNIQUE_newB', + }, + ]), + ); + + setPhysicalIndexes(['legacyA', 'legacyB']); + + renameIndexMock.mockRejectedValueOnce(new Error('boom')); + + const { queryRunner, query, commit, rollback } = buildQueryRunner(); + const dataSource = { + createQueryRunner: () => queryRunner, + } as unknown as DataSource; + + await runOnWorkspace(dataSource); + + expect(query).toHaveBeenCalledWith( + expect.stringContaining('ROLLBACK TO SAVEPOINT'), + ); + // The failed operation's metadata update is rolled back with its savepoint. + expect(query).not.toHaveBeenCalledWith(expect.stringContaining('UPDATE'), [ + 'IDX_UNIQUE_newA', + 'index-a', + WORKSPACE_ID, + ]); + expect(renameIndexMock).toHaveBeenCalledTimes(2); + expect(query).toHaveBeenCalledWith(expect.stringContaining('UPDATE'), [ + 'IDX_UNIQUE_newB', + 'index-b', + WORKSPACE_ID, + ]); + expect(rollback).not.toHaveBeenCalled(); + expect(commit).toHaveBeenCalled(); + expect(invalidateAndRecomputeMock).toHaveBeenCalledWith(WORKSPACE_ID, [ + 'flatIndexMaps', + 'flatObjectMetadataMaps', + ]); + }); + + it('rolls back the whole transaction when savepoint recovery itself fails', async () => { + getOrRecomputeMock.mockResolvedValue( + buildFlatEntityMaps([ + { + universalIdentifier: 'idx-uid', + id: 'index-1', + name: 'legacyhash', + expectedName: 'IDX_UNIQUE_new', + }, + ]), + ); + + setPhysicalIndexes(['legacyhash']); + + renameIndexMock.mockRejectedValueOnce(new Error('boom')); + + const { queryRunner, query, commit, rollback } = buildQueryRunner(); + + query.mockImplementation((sql: string) => + sql.startsWith('ROLLBACK TO SAVEPOINT') + ? Promise.reject(new Error('savepoint recovery failed')) + : Promise.resolve(), + ); + + const dataSource = { + createQueryRunner: () => queryRunner, + } as unknown as DataSource; + + await expect(runOnWorkspace(dataSource)).rejects.toThrow( + 'savepoint recovery failed', + ); expect(rollback).toHaveBeenCalled(); expect(commit).not.toHaveBeenCalled(); diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/__tests__/are-index-definitions-equivalent.util.spec.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/__tests__/are-index-definitions-equivalent.util.spec.ts new file mode 100644 index 0000000000..c479516931 --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/__tests__/are-index-definitions-equivalent.util.spec.ts @@ -0,0 +1,67 @@ +import { areIndexDefinitionsEquivalent } from 'src/database/commands/upgrade-version-command/2-18/utils/are-index-definitions-equivalent.util'; + +describe('areIndexDefinitionsEquivalent', () => { + it('should return true when definitions only differ by index name', () => { + expect( + areIndexDefinitionsEquivalent({ + indexDefinitionA: + 'CREATE UNIQUE INDEX legacyhash ON workspace_test.company USING btree (name)', + indexDefinitionB: + 'CREATE UNIQUE INDEX "IDX_UNIQUE_new" ON workspace_test.company USING btree (name)', + }), + ).toBe(true); + }); + + it('should return true for identical non-unique definitions', () => { + expect( + areIndexDefinitionsEquivalent({ + indexDefinitionA: + 'CREATE INDEX a ON workspace_test.company USING gin ("searchVector")', + indexDefinitionB: + 'CREATE INDEX b ON workspace_test.company USING gin ("searchVector")', + }), + ).toBe(true); + }); + + it('should return false when uniqueness differs', () => { + expect( + areIndexDefinitionsEquivalent({ + indexDefinitionA: + 'CREATE INDEX a ON workspace_test.company USING btree (name)', + indexDefinitionB: + 'CREATE UNIQUE INDEX b ON workspace_test.company USING btree (name)', + }), + ).toBe(false); + }); + + it('should return false when columns differ', () => { + expect( + areIndexDefinitionsEquivalent({ + indexDefinitionA: + 'CREATE INDEX a ON workspace_test.company USING btree (name)', + indexDefinitionB: + 'CREATE INDEX b ON workspace_test.company USING btree ("createdAt")', + }), + ).toBe(false); + }); + + it('should return false when where clauses differ', () => { + expect( + areIndexDefinitionsEquivalent({ + indexDefinitionA: + 'CREATE UNIQUE INDEX a ON workspace_test.company USING btree (name) WHERE ("deletedAt" IS NULL)', + indexDefinitionB: + 'CREATE UNIQUE INDEX b ON workspace_test.company USING btree (name)', + }), + ).toBe(false); + }); + + it('should return false for malformed definitions', () => { + expect( + areIndexDefinitionsEquivalent({ + indexDefinitionA: 'not an index definition', + indexDefinitionB: 'not an index definition', + }), + ).toBe(false); + }); +}); diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/are-index-definitions-equivalent.util.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/are-index-definitions-equivalent.util.ts new file mode 100644 index 0000000000..63f1ed42d6 --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/are-index-definitions-equivalent.util.ts @@ -0,0 +1,28 @@ +import { isDefined } from 'twenty-shared/utils'; + +// pg_indexes.indexdef has the shape `CREATE [UNIQUE] INDEX ON ...`; +// two physical indexes are duplicates when everything but the name matches. +export const areIndexDefinitionsEquivalent = ({ + indexDefinitionA, + indexDefinitionB, +}: { + indexDefinitionA: string; + indexDefinitionB: string; +}): boolean => { + const toComparable = (indexDefinition: string): string | null => { + const onClauseStart = indexDefinition.indexOf(' ON '); + + if (onClauseStart === -1) { + return null; + } + + const isUnique = indexDefinition.startsWith('CREATE UNIQUE INDEX'); + + return `${isUnique ? 'UNIQUE' : 'NON_UNIQUE'}${indexDefinition.slice(onClauseStart)}`; + }; + + const comparableA = toComparable(indexDefinitionA); + const comparableB = toComparable(indexDefinitionB); + + return isDefined(comparableA) && comparableA === comparableB; +}; diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util.ts new file mode 100644 index 0000000000..d49140805c --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/does-physical-index-exist.util.ts @@ -0,0 +1,34 @@ +import { type QueryRunner } from 'typeorm'; + +// Physical catalog introspection is an upgrade-only concern: in normal +// operation metadata is the source of truth and migrations never need to ask +// Postgres whether an index actually exists. Keep this helper scoped to the +// legacy index name normalization command instead of the shared schema +// manager. +// +// Query pg_class/pg_namespace directly instead of the pg_indexes view: the +// view joins pg_index against pg_class twice and derives schemaname from the +// table's namespace, which prevents an index lookup and can scan the whole +// catalog on large multi-tenant clusters. This lookup hits the unique +// (relname, relnamespace) index on pg_class. +export const doesPhysicalIndexExist = async ({ + queryRunner, + schemaName, + indexName, +}: { + queryRunner: QueryRunner; + schemaName: string; + indexName: string; +}): Promise => { + const result: { exists: boolean }[] = await queryRunner.query( + `SELECT EXISTS ( + SELECT 1 + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = $2 AND n.nspname = $1 AND c.relkind IN ('i', 'I') + ) AS "exists"`, + [schemaName, indexName], + ); + + return result[0]?.exists === true; +}; diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util.ts new file mode 100644 index 0000000000..53f58d6439 --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-18/utils/get-physical-index-definition.util.ts @@ -0,0 +1,32 @@ +import { type QueryRunner } from 'typeorm'; + +// Physical catalog introspection is an upgrade-only concern: in normal +// operation metadata is the source of truth and migrations never need to ask +// Postgres what an index actually looks like. Keep this helper scoped to the +// legacy index name normalization command instead of the shared schema +// manager. +// +// Query pg_class/pg_namespace directly instead of the pg_indexes view: the +// view joins pg_index against pg_class twice and derives schemaname from the +// table's namespace, which prevents an index lookup and can scan the whole +// catalog on large multi-tenant clusters. This lookup hits the unique +// (relname, relnamespace) index on pg_class. +export const getPhysicalIndexDefinition = async ({ + queryRunner, + schemaName, + indexName, +}: { + queryRunner: QueryRunner; + schemaName: string; + indexName: string; +}): Promise => { + const result: { indexdef: string }[] = await queryRunner.query( + `SELECT pg_get_indexdef(c.oid) AS "indexdef" + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = $2 AND n.nspname = $1 AND c.relkind IN ('i', 'I')`, + [schemaName, indexName], + ); + + return result[0]?.indexdef ?? null; +};