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

Migrations may fail to run in mysql if sql_require_primary_key is enabled #44411

Closed
Firehed opened this issue Oct 1, 2022 · 5 comments
Closed

Comments

@Firehed
Copy link

Firehed commented Oct 1, 2022

  • Laravel Version: v9.33.0
  • PHP Version: 8.1.18
  • Database Driver & Version: mysql 8.0.28

Description:

When trying to run a DB migration structured like this:

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

class CreateNovaNotificationsTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('somename', function (Blueprint $table) {
            $table->uuid('id')->primary();
            // ...
        });
    }

    // ...
}

If the DB is a mysql server with sql_require_primary_key enabled, the migration fails with the following message:

SQLSTATE[HY000]: General error: 3750 Unable to create or change a table without a primary key, when the system variable 'sql_require_primary_key' is set. Add a primary key to the table or unset this variable to avoid this message. Note that tables without a primary key can cause performance problems in row-based replication, so please consult your DBA before changing this setting. at /var/www/html/vendor/laravel/framework/src/Illuminate/Database/Connection.php:544)

From what I can tell, this appears to run CREATE TABLE (`id` char(36) not null, ...) default character set utf8mb4 collate 'utf8mb4_unicode_ci') and then ALTER TABLE ... ADD PRIMARY KEY (`id`)

Ideally, the two would be combined into a single statement, as $table->id() does. I'm not sure if there are other options (such as temporarily disabling that mode)

Steps To Reproduce:

The easiest repro step is to do a clean install of Laravel Nova and try to run artisan migrate when configured to a MySQL server with that setting enabled (this is, apparently, the default on DigitalOcean managed DBs) - I ran into this when one of the built-in Nova migrations was running. However I believe this is not a Nova issue, but one where any UUID PK is configured with the above structure.

Otherwise, a migration as above should be sufficient.

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Oct 1, 2022

See #33238 and the discussion with the reason why this is not considered a Laravel bug.

There is also a workaround on this comment: #33238 (comment)

@Firehed
Copy link
Author

Firehed commented Oct 1, 2022

Hi @rodrigopedra - that's a helpful start, thanks!

Since the problematic migration is part of Nova, I can't realistically apply that workaround in the migration itself since it's a vendor file. It's probably not appropriate to edit artisan itself either. I'd think editing either could easily result in the workaround being lost during a dependency update (plus it would significantly complicate deployments).

Any thoughts?

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Oct 1, 2022

you can try creating a migration, on your project, with an earlier date, much like the users migration has 000000 in its timestamp, to disable sql_require_primary_key

and then a migration with a date greater than the most recent nova migration to reenable sql_require_primary_key

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Oct 2, 2022

@Firehed yesterday I was on the go, and didn't think of events.

You can also listen for the Illuminate\Console\Events\CommandStarting and Illuminate\Console\Events\CommandFinished events to disable/enable sql_require_primary_key.

Check for the migrate and migrate:fresh commands on their $command property.

@driesvints
Copy link
Member

Thanks @rodrigopedra

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

No branches or pull requests

3 participants