phinx-migrations-generator: Failure on generated columns

Hi Daniel,

I got a table created as

CREATE TABLE t_assets_items (
c_uid int(11) NOT NULL,
c_data json NOT NULL,
stor_name varchar(255) COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS (json_unquote(json_extract(c_data,'$.data.item'))) VIRTUAL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

running the migrations generator, I end up with

        $this->table('t_assets_items', [
                'id' => false,
                'engine' => 'InnoDB',
                'encoding' => 'utf8mb4',
                'collation' => 'utf8mb4_unicode_ci',
                'comment' => '',
                'row_format' => 'DYNAMIC',
            ])
            ->addColumn('c_data', 'json', [
                'null' => false,
            ])
            ->addColumn('c_uid', 'integer', [
                'null' => false,
                'limit' => MysqlAdapter::INT_REGULAR,
                'after' => 'c_data',
            ])
            ->addColumn('stor_name', 'string', [
                'null' => true,
                'limit' => 255,
                'collation' => 'utf8mb4_unicode_ci',
                'encoding' => 'utf8mb4',
                'after' => 'c_uid',
            ])
            ->create();

with the stor_name column ending up with some wrong code. Would you please look into this one? I’m ready to help with tests etc., just let me know.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 19 (9 by maintainers)

Most upvoted comments

That’s nice. I will take a look at it in the next few days.

Absolutely makes sense. I’ve been using phinx mainly because there wasn’t any other maintained database migration tool in the php ecosystem and it was still quite a pain to use until your project appeared.

The things I appreciated about phinx is the change() method while I was still writing migrations manually. Many other things feel unnecessarily abstracted away towards making phinx hard to use in real world.

I’ll do some more tests in the week (with respect to the new feature) & let you know. Raw sql would be imho awesome.

I have come to the conclusion that this tool should only offer features that are also supported by Phinx. So as long as Phinx does not support generated columns, this migration generator will not support it. I tried to generate SQL diffs, but that doesn’t make sense since Phinx should be responsible for that task. So I’m sorry to close this request, but the work and time involved would be too much for me.

Currently I’m trying to add this feature. Your suggested workaround with Literal::from(...) is tricky but it works on my dev machine. The “up” direction works so far, but there are some other problems now:

  • Changing a generated column would not work with the current logic of the migration generator because a generated column must be dropped and then created to change it. All other fields can be changed directly with phinx, but not this kind of field and with the current logic the generator is internally build.
  • Another option would be to generate raw SQL for generated columns, but this would also break a lot of existing internal logic. It would also require adding support for the phinx up and down method, instead of using the change method. So a lot of changes.
  • Phinx does not support generated columns. The PR to support it was closed. So I guess it will “never” be supported directly because phinx tries to abstract multiple databases systems.

So you see I’ve tried it, but this feature needs a lot of more changes under the hood then expected.

Hey @odan , I’d like to reopen this issue. I have a working example on how to add a generated column properly with phinx - its been supported, only not too obviously documented. Here the code:

<?php

use Phinx\Db\Adapter\MysqlAdapter;
use Phinx\Util\Literal;

class Asset extends Phinx\Migration\AbstractMigration
{
    public function change()
    {
        $this->execute('SET unique_checks=0; SET foreign_key_checks=0;');
        $this->table('t_assets_items', [
                'id' => false,
                'primary_key' => ['c_uid'],
                'engine' => 'InnoDB',
                'encoding' => 'utf8mb4',
                'collation' => 'utf8mb4_0900_ai_ci',
                'comment' => 'Assets items',
                'row_format' => 'DYNAMIC',
            ])
            ->addColumn('c_ref1', Literal::from("varchar(255) COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS (`c_json`->>'$.uid') VIRTUAL"), [
                'null' => false,
                'comment' => 'Unique row id',
            ])
            ->update();
        $this->execute('SET unique_checks=1; SET foreign_key_checks=1;');
    }
}

this does actually the correct thing (–dry-run excerpt)

php vendor/bin/phinx migrate -e production --configuration=phinx.yml --dry-run
Phinx by CakePHP - https://phinx.org.

using config file ./phinx.yml
using config parser yml
using migration paths 
 - /srv/varwwwhtml/migrate-test/glued/Core/Install/Migrations
using seed paths 
 - /srv/varwwwhtml/migrate-test/glued/Core/Install/Seeds
using environment production
using adapter mysql
using database glued
ordering by creation time

 == 20201204120000 Asset: migrating
START TRANSACTION;
SET unique_checks=0; SET foreign_key_checks=0;
ALTER TABLE `t_assets_items` ADD `c_ref1` varchar(255) COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS (`c_json`->>'$.uid') VIRTUAL NOT NULL COMMENT 'Unique row id';
SET unique_checks=1; SET foreign_key_checks=1;
COMMIT;
INSERT INTO `phinxlog` (`version`, `migration_name`, `start_time`, `end_time`, `breakpoint`) VALUES ('20201204120000', 'Asset', '2020-12-04 10:19:15', '2020-12-04 10:19:15', 0);
 == 20201204120000 Asset: migrated 0.0164s

would you mind including supporting this on your side yourself?

Did some trial and error. Couldn’t really progress on using the phinx’s literal class towards the wanted result. I asked the phinx devs now. What naturally works is plain sql … after the create() statement, I just added an execute()

// generated code here
 ->create();
$this->execute("ALTER TABLE `t_calendar_sources` DROP `c_flag_deleted`; ALTER TABLE `t_calendar_sources` ADD `c_flag_deleted` boolean GENERATED ALWAYS AS (IF((json_unquote(json_extract(`c_attr`,'$.deleted'))) = 'true', 1, 0)) VIRTUAL NOT NULL;");

So maybe #80 is a solution unless I have some progress here