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

SQLite issue with default 0 columns when migrating db from schema dump since L11 upgrade #51747

Closed
adiachenko opened this issue Jun 8, 2024 · 7 comments · Fixed by #51803
Closed

Comments

@adiachenko
Copy link

adiachenko commented Jun 8, 2024

Laravel Version

11.10.0

PHP Version

8.3.1

Database Driver & Version

SQLite 3.43.2

Description

After upgrade to Laravel 11 we started receiving the following error in tests (using sqlite in memory)

Illuminate\Database\QueryException: SQLSTATE[23000]: Integrity constraint violation: 19 NOT NULL constraint failed: partners.support_priority

It looked like the issue was caused by columns in schema dump file that had default 0 not null clause attached to them.

I tried looking closer into what is going on and comparing it on Laravel 10 and Laravel 11:

$schema = DB::select("PRAGMA table_info(partners);");
dd($schema);

On Laravel 10 this outputs the following for problematic column:

{
    "cid": 13
    "name": "support_priority"
    "type": "INTEGER"
    "notnull": 1
    "dflt_value": "0"
    "pk": 0
}

Meanwhile, on Laravel 11:

{
    "cid": 13
    "name": "support_priority"
    "type": "INTEGER"
    "notnull": 1
    "dflt_value": null
    "pk": 0
}

Note how Laravel 11 version does not default 0. Recreating schema dump file on Laravel 11 did not fix the issue. So I searched and replaced all instances DEFAULT 0 на DEFAULT '0' in schema dump file and, interestingly, the issue was gone. Makes me wonder in Laravel 11 has some weird type coercion issue that equates 0 to null.

Steps To Reproduce

Note that the issue may not be easy to reproduce because when creating the dump itself Laravel (both on 10 or 11) seemingly randomly uses different syntax for different tables: I have some tables with integer not null default '0' and some with INTEGER DEFAULT 0 NOT NULL on similar columns. I am no sure what the correlation here. However, regardless of this, the dump did not cause issue on Laravel 10, but does on Laravel 11.

@staudenmeir
Copy link
Contributor

Hi @adiachenko,
Did you log and compare the Schema::create() queries between Laravel 10 and 11?

@adiachenko
Copy link
Author

I went through our migrations one by one and managed to distill some sort of example to replicate the issue.

Assume we have the following SQL schema dump file:

CREATE TABLE partners (
  id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
  support_priority INTEGER DEFAULT 0 NOT NULL,
);

Then we have the following 2 migrations to run on top of that file:

Schema::table('partners', function (Blueprint $table) {
    $table->unsignedSmallInteger('partner_type')->nullable();
});

Schema::table('partners', function (Blueprint $table) {
    $table->unsignedSmallInteger('partner_type')->default(0)->change();
});

When running the second alter table statement Laravel 11 will execute the following queries:

create table "__temp__partners" (
  "id" integer primary key autoincrement not null,
  "support_priority" integer not null,
  "partner_type" integer not null default '0'
);

drop table "partners";

alter table
  "__temp__partners" rename to "partners";

Note how support_priority is now just not null somehow without any default.

After I manually replaced default 0 with default '0' in schema dump it generated a correct query.

create table "__temp__partners" (
  "id" integer primary key autoincrement not null,
  "support_priority" integer not null default '0',
  "partner_type" integer not null default '0'
)

However, I do not like this "fix", because it will be overridden each time I recreate SQL dump meaning I will need to manually change all of these again.

@staudenmeir
Copy link
Contributor

/cc @hafezdivandari

@hafezdivandari
Copy link
Contributor

@staudenmeir thank you for mentioning me.

The issue seems to be on the following line, causes default 0 to be null:

'default' => $column['default'] ? new Expression($column['default']) : null,

I'm on vacation until Friday, and can't send a PR till then.

Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@sinarahmany
Copy link

A possible workaround is to manually update the schema dump to use DEFAULT '0' instead of DEFAULT 0. However, this isn't ideal as it requires manual intervention each time the schema dump is recreated.

I suggest opening a pull request to address this issue in SQLiteGrammar.php.

@hafezdivandari
Copy link
Contributor

You may check PR #51803

hafezdivandari added a commit to hafezdivandari/framework that referenced this issue Jun 14, 2024
taylorotwell added a commit that referenced this issue Jul 4, 2024
* enhance migrations

* formatting

* support drop column on legacy sqlite

* formatting

* fix tests

* force re-run tests

* formatting

* fix altering a table that has a column with zero as default

fixes #51747

* formatting

* formatting

* formatting

* formatting

* formatting

* Formatting

---------

Co-authored-by: Taylor Otwell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants