backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 38 forks source link

db_change_field(): renaming and re-adding indices in a single step is problematic with some older versions of MariaDB #6554

Open klonos opened 2 weeks ago

klonos commented 2 weeks ago

This issue was discovered while working on #5496. Now that that issue has been closed as works as designed, I'm opening this here to deal with that problem specifically.

I'll switch the WIP PR to point to this issue here, and I'll loop back to this in order to go through the comments in the other issue, and see which ones make sense to be copied here. For now, copying only certain comments that have information about reproducing the problem:

...I have tested this on Pantheon multiple times (before the upstream MariaDB version was upped), using the scenario @irinaz has been using to trigger the issue. But I guess that that doesn't count, since I was the one that provided the PR. The irony here is that now that we've updated the Backdrop recipe for Pantheon, the issue won't be easily reproducible 😅

If people want to test this on their local with lando, know that you can provision the database with a specific version, and both MariaDB and MySQL support that in lando:

This was discussed during the weekly dev meeting. Next steps/actions:

  • confirm that this also applies to outdated MySQL versions (5.5.30)
  • check other instances of db_change_field() where we are renaming tables and adding indices at the same time, and include the same fix in the same PR (language module seems to be doing the same)
  • add a @todo item to perhaps remove/revert the fix once the version on MySQL/MariaDB is officially bumped in Backdrop (at which point a requirement notice will be shown when attempting to use these versions anyway)
  • add a separate issue, to consider removing the option to change indices from db_change_field() (although that would be a BC breaking change, so slated for 2.x)

...noting that @quicksketch mentioned that he has also tested this with DDEV, and have found that the PR fixes the problem and does not cause any new problems (see https://youtu.be/t0o9lxSsTmc?t=2245)

Although the D7 to Backdrop upgrade is only one possible use case to have this problem surface, it seems that it is not necessarily tied to or caused by/during that task.