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

[8.x] Solve the Primary Key issue in databases with sql_require_primary_key enabled #37715

Merged
merged 3 commits into from
Jun 18, 2021

Conversation

khalilst
Copy link
Contributor

@khalilst khalilst commented Jun 17, 2021

This PR created in the following of this issue #33238 & here to solve the migration problem for databases with sql_require_primary_key enabled, (e.g. DigitalOcean machines).

It was OK for the auto-increment fields, but it was not working for other fields which need to be primary, e.g. the Passport Migration:

    $this->schema->create('oauth_auth_codes', function (Blueprint $table) {
        $table->string('id', 100)->primary();
        //Other fields
    });

In this situation, at first, the table was created and then altered to have a primary key on id.
So there were two queries executed to create the above table.

#The first one:
create table `oauth_auth_codes` (`id` varchar(100) not null) default character set utf8mb4 collate 'utf8mb4_unicode_ci';

#The second one:
alter table `oauth_auth_codes` add primary key `users_id_primary`(`id`);

This PR modifies the grammar to produce ONLY SINGLE QUERY like this:

create table `users` (`id` varchar(100) primary key not null) default character set utf8mb4 collate 'utf8mb4_unicode_ci'

This causes a tiny improvement during migrations especially for tests using RefreshDatabase middleware.
Also, the changes include MySQL, Postgres, SQL Server and SQLite with the required TESTS for all grammars.

khalilst added 2 commits June 17, 2021 14:40
Remove the primary item from the fluent indexes and add it to the modifiers
…eating primary key in single query without altering table
@khalilst khalilst force-pushed the migration-foriegn-key branch from aeac8cf to 4935be4 Compare June 17, 2021 10:36
@bjhijmans
Copy link

Does this also work for tables with compound primary keys? Such as:

Schema::create('hobby_user', function (Blueprint $table) {
    $table->uuid('hobby_id');
    $table->uuid('user_id');
    $table->primary(['hobby_id', 'user_id']);
}

I don't know if this can work, but it would make sense to me if the SQL generated in a Schema::create() or Schema::table() call always combined into a single query. That would also have the added bonus that an error in a column definition wouldn't leave a partially created or updated table.

@khalilst
Copy link
Contributor Author

@bjhijmans No. It doesn't work for compound primary keys.
This PR only handles adding the primary key to a single field.
I may work on compound primary keys during another PR.
Also, I have to mention that the only way to create this kind of query is:

    $table->string('id', 100)->primary();

If you call the primary method using blueprint itself (not field), it generates the same ALTER query.

    $table->string('id', 100);
    $table->primary('id');

In support of altering the primary keys.

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.

3 participants