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

bake migration_snapshot bug with binary length #331

Closed
1 of 3 tasks
dereuromark opened this issue Oct 24, 2017 · 4 comments
Closed
1 of 3 tasks

bake migration_snapshot bug with binary length #331

dereuromark opened this issue Oct 24, 2017 · 4 comments
Labels

Comments

@dereuromark
Copy link
Member

This is a (multiple allowed):

  • bug
  • enhancement
  • feature-discussion (RFC)

We got the following DB:

CREATE TABLE `uuid16_test_rows` (
  `id` binary(16) NOT NULL,
  `name` varchar(100) COLLATE utf8_unicode_ci NOT NULL DEFAULT '',
  `foreign_key_without_index` binary(16) DEFAULT NULL,
  `foreign_key_with_index` binary(16) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

ALTER TABLE `uuid16_test_rows`
  ADD PRIMARY KEY (`id`);

After running bin/cake bake migration_snapshot Init I get:

        $this->table('uuid16_test_rows', ['id' => false, 'primary_key' => ['id']])
            ->addColumn('id', 'binary', [
                'default' => null,
                'limit' => null,
                'null' => false,
            ])
            ->addColumn('foreign_key_without_index', 'binary', [
                'default' => null,
                'limit' => null,
                'null' => true,
            ])
            ->addColumn('foreign_key_with_index', 'binary', [
                'default' => null,
                'limit' => null,
                'null' => true,
            ])
            ->create();
    }

But in fact this drops the 16 length.
So on the next DB this will become faulty:

CREATE TABLE `uuid16_test_rows` (
  `id` binary(255) NOT NULL,
  `name` varchar(100) COLLATE utf8_unicode_ci NOT NULL DEFAULT '',
  `foreign_key_without_index` binary(255) DEFAULT NULL,
  `foreign_key_with_index` binary(255) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
...

latest 3.5 and migrations plugin

@dereuromark
Copy link
Member Author

It is weird that CakePHP itself has binaryuuid type for years ( https://github.com/cakephp/cakephp/pull/11297/files#diff-26ba5244f4728ab88ec0f493f66576e2R37 )
But Migrations (and the underlying Phinx) does not know this type yet.

I had to workaround using

$table->addColumn('uuid', 'binary', [
	'limit' => 16,
	'default' => null,
	'null' => false,
]);

for now.

@dereuromark
Copy link
Member Author

dereuromark commented Mar 24, 2020

Given that cakephp/phinx#1320 is enabling Phinx to have a binaryuuid natively, is that the way to go?
Or could we add it to Migrations plugin somehow as intermediate solution?

EDIT
I just checked

InvalidArgumentException: An invalid column type "binaryuuid" was specified for column "id". in /sandbox.local/vendor/robmorgan/phinx/src/Phinx/Db/Table.php:305

It seems the generated migration cannot be understood by Phinx right now. So it seems Bake is fine once we fix Phinx to understand this type.

@markstory
Copy link
Member

Having binaryuuid in phinx seems like the best path forward to me. I would prefer if the migrations plugin was a thin wrapper around phinx so that we don't burn time trying to locate in which layer problems exist.

@dereuromark
Copy link
Member Author

cakephp/phinx#1734 solves it
I just did a bin/cake bake migration_snapshot and it produced the expected output

E.g.

        $this->table('ztable', ['id' => false, 'primary_key' => ['id']])
            ->addColumn('id', 'binaryuuid', [
                'default' => null,
                'limit' => null,
                'null' => false,
            ])
            ->addColumn('user_id', 'integer', [
                'default' => null,
                'limit' => 11,
                'null' => false,
            ])
            ->create();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants