Fixes #3705 - DB migration to gitlab 8.x fails when MySQL clustering and replication is used

Fixes error "CREATE TABLE ... SELECT is forbidden when @@GLOBAL.ENFORCE_GTID_CONSISTENCY = 1." when migrating DB on a MySQL cluster with replication. This issue #3705 (closed) only should affect upgrades to version 8.x of Gitlab.

The DB migrate script uses a CREATE TABLE AS SELECT statement which raises the above error. It is more appropriate in this context (since this is actually a temporary table) to create a temporary table.

This change has no impact to UI and only affects the upgrade process.

I didn't modify the changelog to include this change, because it is so ridiculously trivial. I realize that's technically against the contribution guidelines, but couldn't justify taking credit for such a tiny change. If you absolutely need me to take credit for it, please let me know and I will be happy to resubmit a compliant MR.

As I recall, the TEMPORARY keyword was taken out in a22b6c59 because the MySQL user requires permission to create temporary tables. We may have to revisit that decision.

/cc: @haynes

indeed. this was removed because in the documentation the user didn't have the necessary rights to create a temporary table.
also in that migration the table get's created and is deleted after the migration finished so there isn't really a need for a temporary table. maybe we can split this up in two sql statements?
first create a table and then use something like this?:

INSERT INTO table2
(column_name(s))
SELECT column_name(s)
FROM table1;

According to MySQL manual section on replication GTID restrictions @haynes suggestion might work, since an INSERT from a SELECT is not explicitly mentioned as unsupported under these conditions.

Having said that, perhaps the upgrade guide from 7.x to 8.x should simply be altered instructing the admin to update MySQL permissions? IMO creating a temporary table for this purpose does seem like the proper and cleaner way to do it.

I agree.

Shall I wrap this up in another MR then?

I think the install documentation should be at least updated to include permissions to create temporary tables. I think we have to be careful here with changing the migration: rereading #2590 (closed), I think there was concern that many users won't have this permission set, and so the migration would break users trying to upgrade. You may need to add this to 8.x upgrade documentation as well.

@stanhu I updated the install docs to include that permission when I removed the TEMPORARY keyword.
My suggestion would be to also update the 7x to 8.x update guide and maybe mention that you users should add that permission in the next release blog post as well.
Regardless of that, I think we should avoid using a temporary table in this migration if possible because a lot of people don't have it set yet.

I think this has been fixed, so I'm closing this MR.
Feel free to reopen if I'm wrong.

Status changed to closed

mentioned in commit 466ce3e1

mentioned in commit 191e748c

mentioned in commit 4ad8ed1f

mentioned in commit 29d8b4ee