Phinx: $this->dropTable('my_table'); does not work

Created on 13 Jun 2018  路  10Comments  路  Source: cakephp/phinx

Hello,
I'm trying to setup a simple migration script with up() and down() function.
The up() is working fine but the down() does not do anything.

Short story:

My up() function is creating a table, after ./phinx migrate, then I can see my new table in my database => all is OK
My down function() is deleting the table with $this->dropTable('my_table');, then the table is still present in my database => it's not OK, I want my table to be dropped as I expect the dropTable() is doing that.

Long story:

My migration script looks like:

use Phinx\Migration\AbstractMigration;

class CreateMyTable extends AbstractMigration
{
    /**
     * Migrate Up.
     */
    public function up()
    {
        $this->table('my_table', ['id' => false, 'primary_key' => 'C_Role', 'engine' => 'MyISAM'])
             ->addColumn('C_Role', 'integer', ['identity' => true])
             ->addColumn('T_Role', 'string', ['limit' => 45, 'default' => NULL, 'null' => true])
             ->addColumn('T_Role_Libe', 'string', ['limit' => 255, 'default' => NULL, 'null' => true])
             ->save();

        $rows = [
            [
              'C_Role' => 1,
              'T_Role' => 'CHT',
              'T_Role_Libe' => 'Foo',
            ],
            [
              'C_Role' => 2,
              'T_Role' => 'EXE',
              'T_Role_Libe' => 'Bar',
            ],
        ];

        $this->insert('my_table', $rows);
    }

    /**
     * Migrate Down.
     */
    public function down()
    {
        $this->dropTable('my_table');
    }
}

The command line result looks fine:

me@server:~$ ./phinx migrate
Phinx by CakePHP - https://phinx.org. 0.10.0

using config file ./bin/phinx.php
using config parser php
using migration paths 
 - /bdd/migrations
using seed paths 
 - /bdd/seeds
warning no environment specified, defaulting to: development
using database my_database

 == 20180612082039 CreateMyTable: migrating
 == 20180612082039 CreateMyTable: migrated 0.0723s

All Done. Took 0.0886s
me@server:~$ ./phinx rollback
Phinx by CakePHP - https://phinx.org. 0.10.0

using config file ./bin/phinx.php
using config parser php
using migration paths 
 - /bdd/migrations
using seed paths 
 - /bdd/seeds
warning no environment specified, defaulting to: development
using database my_database
ordering by creation time

 == 20180612082039 CreateMyTable: reverting
 == 20180612082039 CreateMyTable: reverted 0.0069s

All Done. Took 0.0233s

Investigation:

I investigated a bit and following the dropTable function I can understand that nothing append because Phinx never execute the query. More detail:

My code call $this->dropTable('my_table'); ($this refer to AbstractMigration class)
AbstractMigration class implement dropTable() as:
https://github.com/cakephp/phinx/blob/8b838d0216ed926a1c2e5aba906c7933709ed1fd/src/Phinx/Migration/AbstractMigration.php#L316-L325

$this->table() return a Table class
Table class implement drop() as:
https://github.com/cakephp/phinx/blob/8b838d0216ed926a1c2e5aba906c7933709ed1fd/src/Phinx/Db/Table.php#L162-L172

At this point the Table has a new 芦action禄 who should drop the table. But this action is never executed.

In order to get the action to be executed we need to call either Table::create(), Table::update() or Table::executeActions()

My quick fix has been to add ->update() to the dropTable() function like:

public function dropTable($tableName)
    {
        $this->table($tableName)->drop()-update();
    }

With this fix, the rollback drops correctly my table and the table is removed from my database as I expect.

I'm not sure my fix is the correct way to fix this issue but it's just my 2 cent'

Regards,
Clement

Most helpful comment

Err, how does one call save() on dropTable() when it doesn't return anything? The non-execute method right now to drop a table is by doing $this->table('table_name')->drop()->save(); as $this->dropTable('table_name')->save(); throws a null exception.

My vote would be to fix it such that it doesn't break the usage of <0.10.0 where $this->dropTable('table_name'); was allowed as a convenient shortcut of above.

All 10 comments

You need to call save() or update() after calling drop table as you found out. Is there anything we can do to communicate this requirement more clearly?

Err, how does one call save() on dropTable() when it doesn't return anything? The non-execute method right now to drop a table is by doing $this->table('table_name')->drop()->save(); as $this->dropTable('table_name')->save(); throws a null exception.

My vote would be to fix it such that it doesn't break the usage of <0.10.0 where $this->dropTable('table_name'); was allowed as a convenient shortcut of above.

@lorenzo I think the simplest way to communicate this requirement is to update the official documentation ;)
http://docs.phinx.org/en/latest/migrations.html#dropping-a-table

As stated by @MasterOdin I finally stopped to use $this->dropTable() because it doesn't give a valuable help.
Instead I use the good old $this->execute(DROP TABLE my_table) because the function does what it says: it execute - right now - the drop of the table.

@MasterOdin in 0.9 it drops the table immediately. In 0.10, it was changed to that it could be chained with other operations, and also had the same lazy behaviour as all other operations for the table.

@F-JJTH I will add it to the docs, thanks for noticing.

Except $this->dropTable('my_table')->save() is an error as dropTable() returns null and will cause a fatal error if following the docs now:

Fatal error: Uncaught Error: Call to a member function save() on null in /var/www/db/migrations/20180613132113_create_my_table.php on line 40

Error: Call to a member function save() on null in /var/www/db/migrations/20180613132113_create_my_table.php on line 40

Call Stack:
    0.0014     348280   1. {main}() /var/www/vendor/robmorgan/phinx/bin/phinx:0
    0.1612    1666272   2. Phinx\Console\PhinxApplication->run() /var/www/vendor/robmorgan/phinx/bin/phinx:28
    0.2039    1973976   3. Phinx\Console\PhinxApplication->doRun() /var/www/vendor/symfony/console/Application.php:122
    0.2050    1973976   4. Phinx\Console\PhinxApplication->doRun() /var/www/vendor/robmorgan/phinx/src/Phinx/Console/PhinxApplication.php:88
    0.2052    1973976   5. Phinx\Console\PhinxApplication->doRunCommand() /var/www/vendor/symfony/console/Application.php:215
    0.2052    1973976   6. Phinx\Console\Command\Rollback->run() /var/www/vendor/symfony/console/Application.php:858
    0.2055    1979344   7. Phinx\Console\Command\Rollback->execute() /var/www/vendor/symfony/console/Command/Command.php:240
    0.2503    2228824   8. Phinx\Migration\Manager->rollback() /var/www/vendor/robmorgan/phinx/src/Phinx/Console/Command/Rollback.php:130
    0.3477    3378464   9. Phinx\Migration\Manager->executeMigration() /var/www/vendor/robmorgan/phinx/src/Phinx/Migration/Manager.php:527
    0.3479    3378464  10. Phinx\Migration\Manager\Environment->executeMigration() /var/www/vendor/robmorgan/phinx/src/Phinx/Migration/Manager.php:386
    0.3480    3378464  11. CreateMyTable->down() /var/www/vendor/robmorgan/phinx/src/Phinx/Migration/Manager/Environment.php:128

And why do I need to call save() on dropTable() and not insert() given that both accept a table name as a string? It's inconsistent.

@MasterOdin Oh, crap... I was confused with drop() I will change the docs again to reflect that the method is gone

I rephrase myself: that the dropTable() method should not be used. Will also mark that method as deprecated and remove in the next release

Why is that method being removed and insert() staying given that they're both convenience wrappers around the more verbose $this->table('table_name')->...()->save() syntax?

@MasterOdin I would love to remove those too. Would you like to help with that task?

Having this kind of aliases have been a source of bugs that I tried to address with this last release. Having a single place where operations are registered is more maintainable, and also gives phinx the chance to perform optimisations, like batching operations.

I have found that bake migration command for baking a drop table migration is missing the ->save() call, so it does nothing if you run right after creating it.

composer info output (to show package installed versions)

...
cakephp/bake                           1.11.2              Bake plugin for CakePHP 
cakephp/cakephp                        3.8.1               The CakePHP framework
cakephp/migrations                     2.2.0               Database Migration plugin for CakePHP 3.0 based on Phinx
robmorgan/phinx                        0.10.8              Phinx makes it ridiculously easy to manage the database migrations for your PH...
...
Was this page helpful?
0 / 5 - 0 ratings

Related issues

saada picture saada  路  14Comments

wpillar picture wpillar  路  97Comments

JamesTheHacker picture JamesTheHacker  路  15Comments

alex-barylski picture alex-barylski  路  14Comments

ahmarov picture ahmarov  路  27Comments