laravel-doctrine / migrations

Integration with Doctrine2's migrations package for Laravel
MIT License
75 stars 77 forks source link

refresh and reset command do not work for me while using postgres #50

Open isaackearl opened 7 years ago

isaackearl commented 7 years ago

Package version, Laravel version

laravel/framework v5.4.28 The Laravel Framework. laravel-doctrine/orm 1.3.6 A Doctrine ORM bridge for Laravel 5 laravel-doctrine/migrations 1.1.5 Doctrine Migrations for Laravel

Expected behaviour

php artisan doctrine:migrations:reset should reset the database... but instead it drops the migrations table first... ?

Actual behaviour

php artisan doctrine:migrations:reset

  [Doctrine\DBAL\Exception\TableNotFoundException]
  An exception occurred while executing 'ALTER TABLE migrations ENABLE TRIGGER ALL':
  SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "migrations" does not exist

  [Doctrine\DBAL\Driver\PDOException]
  SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "migrations" does not exist

  [PDOException]
  SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "migrations" does not exist

Steps to reproduce the behaviour

setup a new app using postgres as the datasource... create a few entities use php artisan doctrine:migrations:diff to create a migratino migrate the table php artisan doctrine:migrations:migrate then try and reset: It will fail because it seems like it drops the migrations table ? If I look at my database afterward... all my database tables still exist except the migrations table.

Also... reset doesn't something similar.

If I migrate and rollback manually everything works fine... it's only the reset or refresh commands that don't work for me.

NOTE: Did some further testing tonight, and Everything works fine with mysql... but reset, refresh are both broken for Postgres...

dave-redfern commented 7 years ago

I remember running into this issue myself quite a while ago. If I remember correctly it's caused by the migrations table being dropped too early in the reset/refresh. I wrote a separate command that ran doctrine:schema:drop --quiet --force --em=<name> and then added an some custom sql queries through the entity manager connection to remove migrations and the queue tables.

ecgifford92 commented 7 years ago

The team I'm on found this issue today and it looks like for postgres, it's attempting to re-enable triggers on the migrations table after the table's already been dropped. You can see this in the safelyDropTable function in the ResetCommand class -- it makes a call to getCardinalityCheckInstructions which provides the query to re-enable the triggers.

Simply removing the post-drop query may have unintended side effects for other database platforms, which is obviously undesirable. Perhaps replacing the enable query (for postgres only) with null and checking before running the query may fix the issue, but I'm unsure if that's a viable solution considering this is the first time I'm looking at the code. I hope this information helps, though.

AnatolyRugalev commented 6 years ago

IMO in cases with needsTableIsolation == true, enabling cardinality checks back makes no sense

AnatolyRugalev commented 6 years ago

I found that DROP TABLE IF EXISTS {$table} CASCADE statement works perfect for PostgreSQL 10.3.

patrickbrouwers commented 6 years ago

@AnatolyRugalev If it's fixed, can I close this issue?

AnatolyRugalev commented 6 years ago

It is not fixed even after merged PR.

I have to override original command in order to make it work. Looks like we need to implement DROP statement for PostgreSQL in a different way. Disabling cardinality checks doesn't allow to drop table if it has any dependents. The solution is to drop them recursively with DROP TABLE IF EXISTS {$table} CASCADE.

Is it OK if I make a PR with this implementation?

patrickbrouwers commented 6 years ago

Sure go ahead