migrate mapping & messages table names are truncated, can lead to incorrect mapping lookups

Created on 20 January 2017, almost 8 years ago
Updated 3 July 2023, over 1 year ago

Problem/Motivation

Migration IDs are more limited than other configuration IDs, because they are used in map table names with a prefix, and in queries with a table.

My migration crashed like this:

Migration failed with source plugin exception: SQLSTATE[42000]: Syntax[error]
error or access violation: 1059 Identifier name
'my-d8-drupal-db.migrate_map_my_migration_id'

because the combination of { my D8 database name } + 'migrate_map_' + { my migration's ID } was more that 64 characters.

It would be nice if Migrate checked for this and refused to install migrations that will cause this problem.

Proposed resolution

Add a hash of the migration_id at the end of the name and truncated it to be < 64 characters. This approach is from Wim Leers in #15 πŸ› migrate mapping & messages table names are truncated, can lead to incorrect mapping lookups Needs work .

Document somewhere that the table name is of the form

  • { my D8 database name } + 'migrate_map_' + { my migration's ID } + {__bundle_name}+ {hash of migration_id}
  • { my D8 database name } + 'migrate_message_' + { my migration's ID } + {__bundle_name}+ {hash of migration_id}

Remaining tasks

Handle the change of migrate table names in a BC friendly way
Test
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Needs review

Version

11.0 πŸ”₯

Component
MigrationΒ  β†’

Last updated 3 days ago

Created by

πŸ‡¬πŸ‡§United Kingdom joachim

Live updates comments and jobs are added and updated live.
  • Needs architectural review

    The issue is available for high level reviews only. If there is a patch or MR it is not to be set to 'Needs work' for coding standards, minor or nit-pick changes.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡ΏNew Zealand quietone

    I was thinking about #47 πŸ› migrate mapping & messages table names are truncated, can lead to incorrect mapping lookups Needs work , where two migrations are sharing a map table name. While it is rare, it is possible. If a site is in such a situation then changing the tables names could break their migrations. In fact, it would if they are using migration_lookup. A migration using a 'shared' table would start using a new table and thus not have access to the older data. If this was detected we could copy the data to the two new tables and everything should be fine.

    But, maybe a simpler way is to not modify the table names when it is detected that 'shared' tables are in use. When 'shared' tables are detected a flag could be set, in key/value, to indicate that the older method for computing table name should be used. And if that flag is not set then the new method is used.

    Setting to NR for comment on this idea.

  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States mikelutz Michigan, USA

    BAck to NW since there is still work to do and no comments for a month

  • Status changed to Needs review 3 months ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    @mikelutz, before work continues I think we should have direction on how to handle #47. I proposed an idea in #65.

  • πŸ‡«πŸ‡·France duaelfr Montpellier, France

    @quietone on ##6:
    I think I'd like to have two copies of the same table more than having to maintain that legacy way of handling migrate tables.
    If we decided to follow the flag way, one day we would certainly like to deprecate that and prepare a new upgrade path that would be the same as the one we would code today, isn't it?

  • @duaelfr opened merge request.
  • πŸ‡«πŸ‡·France duaelfr Montpellier, France

    Rerolled #53 on HEAD in a MR.
    Fixed phpcs and phpstan issues.
    Updated the upgrade path to clone tables instead of renaming them. (I wish I could use a Merge query for this but... πŸ› Merge queries should allow setting the conditionTable Active )

  • πŸ‡¬πŸ‡§United Kingdom catch

    In other situations like this (cache IDs in the database backend, maybe field/bundle names), we hash the end of the ID if the entire thing is too long. It's not great for readability but it doesn't require an update path.

  • πŸ‡«πŸ‡·France duaelfr Montpellier, France

    @catch #71: This is exactly what this issue is about. The original issue was caused by the fact the tables names were only truncated. Now we hash the end but we still have to handle the existing values, hence the upgrade path.

  • πŸ‡ΊπŸ‡ΈUnited States benjifisher Boston area

    I am worried about how disruptive this change could be. Since both @catch and @daffie have commented on this issue, and neither seems concerned about that, I guess it is OK. But I am adding the tag for a change record (NW for that) and I am adding a release-notes snippet to the issue summary.

  • πŸ‡ΊπŸ‡ΈUnited States benjifisher Boston area
Production build 0.71.5 2024