Framework: Tests broken after upgrade doctrine/dbal to 2.10.3

[hakanersu]

• Laravel Version: v7.27.0
• PHP Version: 7.4.3
• Database Driver & Version: Sqlite

Description:

After upgrading from 7.25.0 to v7.27.0 tests getting broken. Im getting query exceptions when using DatabaseMigrations, RefreshDatabase at same time.

It seems to related to https://github.com/laravel/framework/issues/34091

 SQLSTATE[HY000]: General error: 1 near "0": syntax error (SQL: CREATE TABLE tasks (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, account_id INTEGER NOT NULL, user_id INTEGER DEFAULT NULL, subject VARCHAR(255) NOT NULL COLLATE BINARY, start_date DATETIME NOT NULL, state INTEGER DEFAULT 0 NOT NULL, task_type INTEGER DEFAULT 0 NOT NULL, task_queue_id INTEGER NOT NULL, created_by_type VARCHAR(255) DEFAULT NULL COLLATE BINARY, created_by_id INTEGER DEFAULT NULL, created_at DATETIME DEFAULT NULL, updated_at DATETIME DEFAULT NULL, content CLOB DEFAULT NULL COLLATE BINARY, end_date DATETIME DEFAULT NULL, CONSTRAINT 0 FOREIGN KEY (user_id) REFERENCES users (id) ON UPDATE NO ACTION ON DELETE SET NULL NOT DEFERRABLE INITIALLY IMMEDIATE, CONSTRAINT 1 FOREIGN KEY (account_id) REFERENCES accounts (id) ON UPDATE NO ACTION ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE))

  at vendor/laravel/framework/src/Illuminate/Database/Connection.php:671
    667|         // If an exception occurs when attempting to run a query, we'll format the error
    668|         // message to include the bindings with SQL, which will make this exception a
    669|         // lot more helpful to the developer instead of just the database's errors.
    670|         catch (Exception $e) {
  > 671|             throw new QueryException(
    672|                 $query, $this->prepareBindings($bindings), $e
    673|             );
    674|         }
    675| 

  1   vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:72
      Doctrine\DBAL\Driver\PDOException::("SQLSTATE[HY000]: General error: 1 near "0": syntax error")

  2   vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:67
      PDOException::("SQLSTATE[HY000]: General error: 1 near "0": syntax error")

  3   vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:67
      PDO::prepare()

  4   vendor/laravel/framework/src/Illuminate/Database/Connection.php:458
      Doctrine\DBAL\Driver\PDOConnection::prepare()

  5   vendor/laravel/framework/src/Illuminate/Database/Connection.php:664
      Illuminate\Database\Connection::Illuminate\Database\{closure}()

  6   vendor/laravel/framework/src/Illuminate/Database/Connection.php:631
      Illuminate\Database\Connection::runQueryCallback()

  7   vendor/laravel/framework/src/Illuminate/Database/Connection.php:465
      Illuminate\Database\Connection::run()

  8   vendor/laravel/framework/src/Illuminate/Database/Schema/Blueprint.php:102
      Illuminate\Database\Connection::statement()

  9   vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php:290
      Illuminate\Database\Schema\Blueprint::build()

  10  vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php:151
      Illuminate\Database\Schema\Builder::build()

  11  vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php:261
      Illuminate\Database\Schema\Builder::table()

  12  database/migrations/2020_05_04_085002_add_nullable_to_some_fields_in_tasks_table.php:19
      Illuminate\Support\Facades\Facade::__callStatic()

  13  vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:392
      AddNullableToSomeFieldsInTasksTable::up()

  14  vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:401
      Illuminate\Database\Migrations\Migrator::Illuminate\Database\Migrations\{closure}()

  15  vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:200
      Illuminate\Database\Migrations\Migrator::runMigration()

  16  vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:165
      Illuminate\Database\Migrations\Migrator::runUp()

  17  vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:110
      Illuminate\Database\Migrations\Migrator::runPending()

  18  vendor/laravel/framework/src/Illuminate/Database/Console/Migrations/MigrateCommand.php:72
      Illuminate\Database\Migrations\Migrator::run()

  19  vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:541
      Illuminate\Database\Console\Migrations\MigrateCommand::Illuminate\Database\Console\Migrations\{closure}()

  20  vendor/laravel/framework/src/Illuminate/Database/Console/Migrations/MigrateCommand.php:81
      Illuminate\Database\Migrations\Migrator::usingConnection()

  21  vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:37
      Illuminate\Database\Console\Migrations\MigrateCommand::handle()

  22  vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:37
      call_user_func_array()

  23  vendor/laravel/framework/src/Illuminate/Container/Util.php:37
      Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()

  24  vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:95
      Illuminate\Container\Util::unwrapIfClosure()

  25  vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:39
      Illuminate\Container\BoundMethod::callBoundMethod()

  26  vendor/laravel/framework/src/Illuminate/Container/Container.php:596
      Illuminate\Container\BoundMethod::call()

  27  vendor/laravel/framework/src/Illuminate/Console/Command.php:134
      Illuminate\Container\Container::call()

  28  vendor/symfony/console/Command/Command.php:258
      Illuminate\Console\Command::execute()

  29  vendor/laravel/framework/src/Illuminate/Console/Command.php:121
      Symfony\Component\Console\Command\Command::run()

  30  vendor/symfony/console/Application.php:916
      Illuminate\Console\Command::run()

  31  vendor/symfony/console/Application.php:264
      Symfony\Component\Console\Application::doRunCommand()

  32  vendor/symfony/console/Application.php:140
      Symfony\Component\Console\Application::doRun()

  33  vendor/laravel/framework/src/Illuminate/Console/Application.php:93
      Symfony\Component\Console\Application::run()

  34  vendor/laravel/framework/src/Illuminate/Console/Application.php:185
      Illuminate\Console\Application::run()

  35  vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:263
      Illuminate\Console\Application::call()

  36  vendor/laravel/framework/src/Illuminate/Testing/PendingCommand.php:171
      Illuminate\Foundation\Console\Kernel::call()

<?php

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

class AddNullableToSomeFieldsInTasksTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::table('tasks', function (Blueprint $table) {
            $table->text('content')->nullable()->change();
            $table->dateTime('end_date')->nullable()->change();
        });
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {
        Schema::table('tasks', function (Blueprint $table) {
            $table->text('content')->change();
            $table->dateTime('end_date')->change();
        });
    }
}

Downgrading doctrine/dbal from 2.10.3 to 2.10.2 fixes problem.

Steps To Reproduce:

Upgrade 7.27.0 and create any basic test with DatabaseMigrations, RefreshDatabase. If i use just one of them tests works fine.

[uuf6429]

I have the same problem and spent some time investigating it.

What happened is that \Doctrine\DBAL\Platforms\SqlitePlatform::supportsForeignKeyConstraints has been changed to true in 2.10.3 (if you set this to true on 2.10.2, the same problems happens).

I don't know why, but it seems that doctrine/dbal does not create indexes when creating a table with foreign keys, but when changing an existing table with foreign keys, it tries to remove some non-existent index.

[MyIgel]

Can cofirm that, the https://github.com/doctrine/dbal/commit/85a983c3f8d6447c4441832477def1c28d57bee9 commit is the one that breaks it in dbal for sqlite, so it does not work after an upgrade from dbal v2.10.2 to v2.10.3

[SirNarsh]

Facing a similar problem here also after upgrading, seems not directly laravel but doctrine/dbal 2.10.3, switching to v2.10.2 fixed test failing sqlite failing:
General error: 1 no such index: IDX_DC8C84A116FE72E1 (SQL: DROP INDEX IDX_DC8C84A116FE72E1)

PHP 7.4.8
laravel/framework: v7.27.0
doctrine/dbal 2.10.3

[taylorotwell]

Feel free to share your information on the Doctrine issue I created: https://github.com/doctrine/dbal/issues/4243

[greg0ire]

@hakanersu there are 2 exceptions in the code:

https://github.com/laravel/framework/blob/caec40ed2695c2aa84b5eb195c822e3a961fc445/src/Illuminate/Database/Connection.php#L670-L672

The one that appears in your screenshots comes from Laravel, and is triggered by the one line 670 , that is issued by doctrine/dbal.

When posting an exception you should:

• prefer text over screenshots so that people can copy paste from them, and to be more friendly to search engines;
• include all exceptions, not just the first one.

If you still have access to the stack trace of the second exception, please post it _as text_ on the page linked by @taylorotwell above, it will be helpful :pray:

[greg0ire]

I don't know why, but it seems that doctrine/dbal does not create indexes when creating a table with foreign keys

@uuf6429 was the table created with the latest version of doctrine/dbal?

[uuf6429]

I don't know why, but it seems that doctrine/dbal does not create indexes when creating a table with foreign keys

@uuf6429 was the table created with the latest version of doctrine/dbal?

Yes - this happens before running tests, when running migrations through artisan. The migrations are all based on laravel blueprint.

[greg0ire]

Is this method used at any point, and if yes, is $name null?

https://github.com/laravel/framework/blob/caec40ed2695c2aa84b5eb195c822e3a961fc445/src/Illuminate/Database/Schema/Blueprint.php#L537-L550

Shouldn't that result in

CREATE TABLE tasks (
    id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, 
    account_id INTEGER NOT NULL, 
    user_id INTEGER DEFAULT NULL, 
    subject VARCHAR(255) NOT NULL COLLATE BINARY, 
    start_date DATETIME NOT NULL, 
    state INTEGER DEFAULT 0 NOT NULL, 
    task_type INTEGER DEFAULT 0 NOT NULL, 
    task_queue_id INTEGER NOT NULL, 
    created_by_type VARCHAR(255) DEFAULT NULL COLLATE BINARY, 
    created_by_id INTEGER DEFAULT NULL, 
    created_at DATETIME DEFAULT NULL, 
    updated_at DATETIME DEFAULT NULL, 
    content CLOB DEFAULT NULL COLLATE BINARY, 
    end_date DATETIME DEFAULT NULL, 
-    CONSTRAINT 0 FOREIGN KEY (user_id) REFERENCES users (id) ON UPDATE NO ACTION ON DELETE SET NULL NOT DEFERRABLE INITIALLY IMMEDIATE, 
-    CONSTRAINT 1 FOREIGN KEY (account_id) REFERENCES accounts (id) ON UPDATE NO ACTION ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE
+    FOREIGN KEY (user_id) REFERENCES users (id) ON UPDATE NO ACTION ON DELETE SET NULL NOT DEFERRABLE INITIALLY IMMEDIATE, 
+    FOREIGN KEY (account_id) REFERENCES accounts (id) ON UPDATE NO ACTION ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE
)

Note that this syntax does not even appear in SQlite's official doc about foreign keys

Here is where that syntax might come from: https://github.com/doctrine/dbal/blob/f69c990e359931753232a81c3a1821a7ea060bfd/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php#L2586-L2588

[taylorotwell]

@greg0ire

Here is the actual SQL created when I create a table with a foreign key using Laravel:

create table "posts" 
("id" integer not null primary key autoincrement, 
"user_id" integer not null, 
"body" varchar not null, 
"created_at" datetime null, 
"updated_at" datetime null, 
foreign key("user_id") references "users"("id"))

However, if I later try to use doctrine/dbal to modify that table - such as changing "body" from varchar to text - I run into the exception noted above.

$name is not used when building foreign keys with SQLite. I believe it is used with MySQL or other databases that support it.

[taylorotwell]

@greg0ire

When attempting to change the column, here is an array of SQL statements doctrine/dbal informs us should be run to make the change on SQLite:

array:7 [
  0 => "DROP INDEX IDX_885DBAFAA76ED395"
  1 => "CREATE TEMPORARY TABLE __temp__posts AS SELECT id, user_id, body, created_at, updated_at FROM posts"
  2 => "DROP TABLE posts"
  3 => "CREATE TABLE posts (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, user_id INTEGER NOT NULL, created_at DATETIME DEFAULT NULL, updated_at DATETIME DEFAULT NULL, body CLOB NOT NULL COLLATE BINARY, CONSTRAINT 0 FOREIGN KEY (user_id) REFERENCES users (id) ON UPDATE NO ACTION ON DELETE NO ACTION NOT DEFERRABLE INITIALLY IMMEDIATE)"
  4 => "INSERT INTO posts (id, user_id, body, created_at, updated_at) SELECT id, user_id, body, created_at, updated_at FROM __temp__posts"
  5 => "DROP TABLE __temp__posts"
  6 => "CREATE INDEX IDX_885DBAFAA76ED395 ON posts (user_id)"
]

[greg0ire]

Thanks, it explains a lot! I didn't understand why we were reading information from posts before creating it, didn't know it was dropped then re-created instead of altered.

@MyIgel contributed https://github.com/doctrine/dbal/pull/4246 yesterday, and I think it fixes the "reading information from posts before dropping it" part.

Can people here please test doctrine/[email protected] and report if it does fix the issue for them?

[taylorotwell]

@greg0ire

I installed it (Upgrading doctrine/dbal (2.10.3 => 2.10.x-dev cd2ddda)).

I still receive the same error:

General error: 1 no such index: IDX_885DBAFAA76ED395 (SQL: DROP INDEX IDX_885DBAFAA76ED395)

Verified the change from that PR in my vendor directory so I do appear to be using 2.10.x-dev.

[greg0ire]

Ok so there are in facts 2 bugs? One with query number 0 and one with query number 3, and I think @MyIgel fixed the one with query number 3 with doctrine/dbal#4246 . No idea why the DBAL thinks there should be an index in addition to the foreign key yet.

Will try the reproducer in https://github.com/doctrine/dbal/issues/4243#issuecomment-686340932 later, but feel free to beat me to it, anyone.

EDIT: Started an investigation here: https://github.com/doctrine/dbal/issues/4243#issuecomment-686616962

[uuf6429]

By the way, a bit late, but I have a further suggestion (which worked in my case).

Depending on how many laravel users have this problem, we could add the following to laravel/framework's composer.json, if it doesn't happen often, this could also be done for individual use-cases:

    "conflict": {
        "doctrine/dbal": "2.10.3"
    },

[greg0ire]

For the moment I have this workaround/experiment that is ugly but works: https://github.com/greg0ire/dbal/pull/3
I think the issue could also be fixed in Laravel by making the thing that creates foreign key (so Blueprint?) also create indices on foreign keys (that should take care of issue with query number 0), and by making sure foreign keys are named (by using the CONSTRAINT blah FOREIGN KEY… syntax) (that should take care of issue with query number 3). Not saying the issue should be solved only in Laravel, but I can foresee a debate coming about what the correct behavior for the DBAL should be whereas, the 2 changes in Laravel pointed above might be less subject to debate.

[serhatculhalikp]

@greg0ire this may be related as well or coincidence?
https://github.com/symfony/symfony/issues/38067

[greg0ire]

Too early to tell, but it's possible that it's at least related to the same pull request. Thanks for linking to that, I will keep an eye on it.

[cretueusebiu]

Adding <server name="DB_FOREIGN_KEYS" value="(false)"/> in phpunit.xml fixed the tests for me.

[uuf6429]

Thanks, it explains a lot! I didn't understand why we were reading information from posts before creating it, didn't know it was dropped then re-created instead of altered.

@MyIgel contributed doctrine/dbal#4246 yesterday, and I think it fixes the "reading information from posts before dropping it" part.

Can people here please test doctrine/[email protected] and report if it does fix the issue for them?

A little context that might help, I learned recently that sqlite does not allow alter statements, so DBALs tend to recreate the table with the new structure.

[ltribolet]

doctrine/dbal (2.10.x-dev 03bc93b) fixes it for me.

[greg0ire]

We have released the bugfix as 2.10.4, please confirm the bug is fixed and close this.

[driesvints]

Awesome, thanks.

[greg0ire]

You're very welcome, sorry for the trouble actually, and thanks to the Laravel community for helping out with the reproducers, PRs and investigations :)

[driesvints]

@greg0ire no sorry needed. Thanks for all your work 👍