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

Postgres String Primary Key Migration Fail #37820

Closed
ahawlitschek opened this issue Jun 25, 2021 · 10 comments · Fixed by #37826
Closed

Postgres String Primary Key Migration Fail #37820

ahawlitschek opened this issue Jun 25, 2021 · 10 comments · Fixed by #37826
Labels

Comments

@ahawlitschek
Copy link

ahawlitschek commented Jun 25, 2021

  • Laravel Version: 8.48.1
  • PHP Version: 8.0.6
  • Database Driver & Version: postgres (PostgreSQL) 13.2 with PostGIS 3

Description:

After Updating to the newest Laravel Version (previously 8.47.0), the default Session migration from Laravel Jetstream causes the following exception:

SQLSTATE[42P16]: Invalid table definition: 7 ERROR: multiple primary keys for table "sessions" are not allowed (SQL: alter table "sessions" add primary key ("id"))

Other migrations with integer primary keys are fully fine.
It seems like the problem is caused by the string primary key.
Here is the migration from jetstream:

Schema::create('sessions', function (Blueprint $table) {
            $table->string('id')->primary();
            $table->foreignId('user_id')->nullable()->index();
            $table->string('ip_address', 45)->nullable();
            $table->text('user_agent')->nullable();
            $table->text('payload');
            $table->integer('last_activity')->index();
        });

We tried this on several machines from different people working on the project.

Steps To Reproduce:

We tried to reproduce this issue with a completely new laravel jetstream project. But the error does not appear.
I will try to create a demo project the next days

@driesvints
Copy link
Member

Most likely caused by #37715 as well.

@khalilst
Copy link
Contributor

@driesvints I have created a project connected to `Postgres.
And put the above migration there.
It was working without any problem!!!

My guess there is something missing in the migration code like this:

Schema::create('sessions', function (Blueprint $table) {
            $table->string('id')->primary();
            //...
            $table->primary('id);
        });

And the error was obvious here and not related to #37715 .

@driesvints
Copy link
Member

@khalilst how do you explain this working in v8.47.0 then? It broke after v8.48.0 and that PR is the only database related change.

@khalilst
Copy link
Contributor

@driesvints, brief answer: probably wrong bug report.

For more clarification:

  1. Install a fresh Laravel project
  2. Create a migration with the following code.
  3. Connect to Postgres and migrate.
  4. It was working without any error.

@ahawlitschek Please provide steps to reproduce to help me to fix it, no need for the demo project.

@ahawlitschek
Copy link
Author

ahawlitschek commented Jun 25, 2021

So, I created another a fresh instance and tested each other dependency from our project one by one until I have found the dependency that need to be added in order to achieve this behaviour.

Add
"mstaack/laravel-postgis": "^5.2", to the dependencies of the fresh in composer.json and it works with laravel 8.47.0, but not with 8.48.0 and 8.48.1 framework.

@driesvints
Copy link
Member

@khalilst the OP's migration is the base sessions table that ships with Laravel: https://github.com/laravel/framework/blob/8.x/src/Illuminate/Session/Console/stubs/database.stub

@khalilst
Copy link
Contributor

@driesvints I have tried it in Laravel 8.48.1 and it was OK,

@ahawlitschek Just let me fix it.

@studnitz
Copy link

There is also an issue and PR at the bosnadev/database repository, which is what mstaack/laravel-postgis depends on.

bosnadev/database#48
bosnadev/database#49

@driesvints
Copy link
Member

Thanks @studnitz. I've sent in a PR to revert all of this: #37826

@khalilst
Copy link
Contributor

As @studnitz said:
This bug related to the third-party package overriding the Blueprint and there is a PR to fix this problem in that package.

The package name: bosnadev/database
File path: Bosnadev/Database/Schema/Blueprint.php
It is required by mstaack/laravel-postgis.

I think this PR #37826 is not fair because of a third-party package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants