fix(server): prevent enum migration failure for long identifier names (#21748)
Fixes https://github.com/twentyhq/twenty/issues/20524 ## Problem Adding (or renaming/removing) an option on a `SELECT` / `MULTI_SELECT` field failed for fields whose object + field name combination is long, surfacing to the user only as: > Migration action 'update' for 'fieldMetadata' failed — Migration execution failed. The real underlying Postgres error was: ``` type "_personalInsurancePolicyOrQuote_insuranceCoverageClassification" already exists ALTER TYPE "..."."_personalInsurancePolicyOrQuote_insuranceCoverageClassifications_enum" RENAME TO "..._insuranceCoverageClassifications_enum_old" ``` ## Root cause PostgreSQL truncates identifiers to **63 bytes** (`NAMEDATALEN - 1`). The enum type name `_personalInsurancePolicyOrQuote_insuranceCoverageClassifications_enum` is **69 chars**, so it was already stored truncated to 63 (`..._insuranceCoverageClassification` — the `_enum` suffix chopped off). Multi-select / select option changes go through the rename-and-recreate path in `alterEnumValues`, which renames the enum to `<name>_old`. That candidate is 73 chars → Postgres truncates it back to the **same 63-byte string** as the source → `type "..." already exists`. The failure is deterministic, so every retry on that field failed. The transaction rolls back cleanly, leaving no `_old` artifacts behind. The temporary column name (`<column>_old`) had the same latent bug for very long field names. ## Fix Add `buildTemporaryIdentifier(base, suffix)` to `WorkspaceSchemaEnumManagerService`, which trims the base name so the `_old` suffix survives within 63 bytes and stays distinct from the original. Applied to both the temporary enum name and the temporary column name. The `_old` type/column are transient (dropped within the same transaction), so the trimmed name only needs to fit and not collide — which it now does. ## Test Added `workspace-schema-enum-manager.service.spec.ts` reproducing the exact failing object/field names. Both assertions (target identifier ≤ 63 bytes; truncated source ≠ truncated target) fail on `main` and pass with the fix. ## Recovery No manual cleanup needed for affected workspaces — failed migrations rolled back cleanly. Once deployed, option edits on long-named fields work; the field remained fully usable in the meantime (only option changes were blocked).
This commit is contained in:
+105
@@ -0,0 +1,105 @@
|
||||
import { type QueryRunner } from 'typeorm';
|
||||
|
||||
import { WorkspaceSchemaEnumManagerService } from 'src/engine/twenty-orm/workspace-schema-manager/services/workspace-schema-enum-manager.service';
|
||||
import { type WorkspaceSchemaColumnDefinition } from 'src/engine/twenty-orm/workspace-schema-manager/types/workspace-schema-column-definition.type';
|
||||
|
||||
// PostgreSQL truncates identifiers to 63 bytes (NAMEDATALEN - 1).
|
||||
const POSTGRES_MAX_IDENTIFIER_LENGTH = 63;
|
||||
|
||||
// Extracts every double-quoted identifier from a SQL string.
|
||||
const getQuotedIdentifiers = (sql: string): string[] =>
|
||||
[...sql.matchAll(/"([^"]+)"/g)].map((match) => match[1]);
|
||||
|
||||
describe('WorkspaceSchemaEnumManagerService', () => {
|
||||
let service: WorkspaceSchemaEnumManagerService;
|
||||
let queryRunner: jest.Mocked<
|
||||
Pick<QueryRunner, 'query' | 'isTransactionActive'>
|
||||
>;
|
||||
let executedSql: string[];
|
||||
|
||||
beforeEach(() => {
|
||||
service = new WorkspaceSchemaEnumManagerService();
|
||||
executedSql = [];
|
||||
queryRunner = {
|
||||
isTransactionActive: true,
|
||||
query: jest.fn((sql: string) => {
|
||||
executedSql.push(sql);
|
||||
|
||||
return Promise.resolve();
|
||||
}),
|
||||
} as unknown as jest.Mocked<
|
||||
Pick<QueryRunner, 'query' | 'isTransactionActive'>
|
||||
>;
|
||||
});
|
||||
|
||||
describe('alterEnumValues', () => {
|
||||
it('should not generate an enum rename whose target collides with the source when the name exceeds the identifier length limit', async () => {
|
||||
// Real-world case: long object + field names produce an enum name over
|
||||
// 63 bytes. Naively appending `_old` truncates the suffix away and the
|
||||
// RENAME target collides with the existing type.
|
||||
const columnDefinition: WorkspaceSchemaColumnDefinition = {
|
||||
name: 'insuranceCoverageClassifications',
|
||||
type: 'enum',
|
||||
isArray: true,
|
||||
isNullable: true,
|
||||
};
|
||||
|
||||
await service.alterEnumValues({
|
||||
queryRunner: queryRunner as unknown as QueryRunner,
|
||||
schemaName: 'workspace_adhj7eaegq93fzpgbfpdm8ok3',
|
||||
tableName: '_personalInsurancePolicyOrQuote',
|
||||
columnDefinition,
|
||||
enumValues: ['OPTION_1', 'OPTION_2', 'OPTION_3'],
|
||||
oldToNewEnumOptionMap: {},
|
||||
});
|
||||
|
||||
const renameStatement = executedSql.find(
|
||||
(sql) => sql.includes('ALTER TYPE') && sql.includes('RENAME TO'),
|
||||
);
|
||||
|
||||
expect(renameStatement).toBeDefined();
|
||||
|
||||
const [sourceEnum, targetEnum] = getQuotedIdentifiers(
|
||||
renameStatement as string,
|
||||
).slice(-2);
|
||||
|
||||
expect(targetEnum.length).toBeLessThanOrEqual(
|
||||
POSTGRES_MAX_IDENTIFIER_LENGTH,
|
||||
);
|
||||
// Once Postgres truncates both to 63 bytes they must still differ,
|
||||
// otherwise the rename targets the type's own name.
|
||||
expect(targetEnum.slice(0, POSTGRES_MAX_IDENTIFIER_LENGTH)).not.toEqual(
|
||||
sourceEnum.slice(0, POSTGRES_MAX_IDENTIFIER_LENGTH),
|
||||
);
|
||||
});
|
||||
|
||||
it('should keep every emitted identifier within the Postgres length limit for long names', async () => {
|
||||
const columnDefinition: WorkspaceSchemaColumnDefinition = {
|
||||
name: 'insuranceCoverageClassifications',
|
||||
type: 'enum',
|
||||
isArray: true,
|
||||
isNullable: true,
|
||||
};
|
||||
|
||||
await service.alterEnumValues({
|
||||
queryRunner: queryRunner as unknown as QueryRunner,
|
||||
schemaName: 'workspace_adhj7eaegq93fzpgbfpdm8ok3',
|
||||
tableName: '_personalInsurancePolicyOrQuote',
|
||||
columnDefinition,
|
||||
enumValues: ['OPTION_1', 'OPTION_2'],
|
||||
oldToNewEnumOptionMap: { OPTION_1: 'OPTION_1' },
|
||||
});
|
||||
|
||||
const temporaryIdentifiers = executedSql
|
||||
.flatMap(getQuotedIdentifiers)
|
||||
.filter((identifier) => identifier.endsWith('_old'));
|
||||
|
||||
expect(temporaryIdentifiers.length).toBeGreaterThan(0);
|
||||
temporaryIdentifiers.forEach((identifier) =>
|
||||
expect(identifier.length).toBeLessThanOrEqual(
|
||||
POSTGRES_MAX_IDENTIFIER_LENGTH,
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
+15
-2
@@ -12,6 +12,19 @@ import {
|
||||
escapeLiteral,
|
||||
} from 'src/engine/workspace-manager/workspace-migration/utils/remove-sql-injection.util';
|
||||
|
||||
const POSTGRES_MAX_IDENTIFIER_LENGTH = 63;
|
||||
|
||||
const buildTemporaryIdentifier = (baseName: string, suffix: string): string => {
|
||||
const maxBaseLength = POSTGRES_MAX_IDENTIFIER_LENGTH - suffix.length;
|
||||
|
||||
const truncatedBase =
|
||||
baseName.length <= maxBaseLength
|
||||
? baseName
|
||||
: baseName.slice(0, maxBaseLength);
|
||||
|
||||
return `${truncatedBase}${suffix}`;
|
||||
};
|
||||
|
||||
export class WorkspaceSchemaEnumManagerService {
|
||||
async createEnum({
|
||||
queryRunner,
|
||||
@@ -150,7 +163,7 @@ export class WorkspaceSchemaEnumManagerService {
|
||||
columnName,
|
||||
});
|
||||
|
||||
const oldEnumName = `${enumName}_old`;
|
||||
const oldEnumName = buildTemporaryIdentifier(enumName, '_old');
|
||||
|
||||
await this.renameEnum({
|
||||
queryRunner,
|
||||
@@ -166,7 +179,7 @@ export class WorkspaceSchemaEnumManagerService {
|
||||
values: enumValues,
|
||||
});
|
||||
|
||||
const oldColumnName = `${columnName}_old`;
|
||||
const oldColumnName = buildTemporaryIdentifier(columnName, '_old');
|
||||
|
||||
await this.renameColumn({
|
||||
queryRunner,
|
||||
|
||||
Reference in New Issue
Block a user