Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix enumMigration not working on long fieldNames #8708

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

charlesBochet
Copy link
Member

Issue

Some users were facing issues while updating SELECT or MULTISELECT options:

Error executing migration QueryFailedError: column "undefined" does not exist
    at PostgresQueryRunner.query (/app/node_modules/typeorm/driver/postgres/PostgresQueryRunner.js:219:19)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async WorkspaceMigrationEnumService.migrateEnumValues (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.js:101:13)
    at async WorkspaceMigrationEnumService.alterEnum (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.js:54:9)
    at async WorkspaceMigrationRunnerService.alterColumn (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.js:254:13)
    at async WorkspaceMigrationRunnerService.handleColumnChanges (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.js:202:21)
    at async WorkspaceMigrationRunnerService.handleTableChanges (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.js:94:25)
    at async WorkspaceMigrationRunnerService.executeMigrationFromPendingMigrations (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.js:60:17)
    at async WorkspaceExecutePendingMigrationsCommand.run (/app/packages/twenty-server/dist/src/engine/workspace-manager/workspace-migration-runner/commands/workspace-execute-pending-migrations.command.js:24:9)
    at async Command.<anonymous> (/app/node_modules/nest-commander/src/command-runner.service.js:162:24) {
  query: '\n' +
    '        UPDATE "workspace_7i7bwawp7keh3gel1g69jropc"."_funnelInProductsRu"\n' +
    '        SET "reasonForStageTransition" = undefined\n' +
    "        WHERE id='2af8db61-5f75-46de-8b1a-97e312937e06'\n" +
    '      ',
  parameters: undefined,
  driverError: error: column "undefined" does not exist

Root cause

Internally, while migrating enum values, we are doing:

const values = await queryRunner.query(
      `SELECT id, "${oldColumnName}" FROM "${schemaName}"."${tableName}"`,
    );

 let val = value[oldColumnName];

oldColumnName being: ${columnDefinition.columnName}_old_${v4()};

However, TypeORM only supports 63 bits identifiers: typeorm/typeorm#3118, but silently truncate the identifer so if oldColumnName gets larger than 63 characters, value[oldColumnName] is undefined.

Fix

We only need a temporary and unique temporary name, no need to take into account the previous columnName here

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR modifies the enum migration process to handle long field names by simplifying the temporary column naming convention, preventing failures due to TypeORM's 63-character identifier limit.

  • Changed temporary column naming in workspace-migration-enum.service.ts from ${columnName}_old_${uuid} to just old_${uuid}
  • Fixed undefined column errors during enum migrations by ensuring temporary column names stay under TypeORM's 63-character limit
  • Maintains backward compatibility while simplifying the migration process for enum fields
  • Resolves user-reported issues with SELECT and MULTISELECT option updates failing

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. LGTM

@charlesBochet charlesBochet merged commit 08852dd into main Nov 24, 2024
19 checks passed
@charlesBochet charlesBochet deleted the fix-temporary-col-name-larger-than-63 branch November 24, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants