Feature Request: custom data types

[bradynpoulsen]

I think it would be really useful to have a way to add a custom column name data type when using addColumn(). This would then make it so it could be aliased to an actual data type.

For example with MySQL,

without custom type

$table->addColumn('id', 'binary', ['limit' => 16]); // uuid v4

with custom type

$table->addColumn('id', 'uuid');

[bradynpoulsen]

Here is a possible flow:

• 1. Register 'uuid' to my UuidCustomType class that implements CustomTypeInterface via
  • a. YAML entry
  • b. Inside a custom migration base class on construct
• 2. Implement UuidCustomType that aliases to the real 'binary' type with ['limit' => 16]

[twoixter]

@bradynpoulsen This is actually something I was thinking about implementing. This semantic is very much like a User Defined Type or a Data Domain. I was skeptical at first because each database has a concept of UDT or Domain (E.G: Check constraints and UDT in SQL Server) and AFAIK, don't supported in migrations. My concern was that if implemented, it would give the impression to extend the user defined type to the DB Adapter.

This is my suggestion for the flow. If interested, I might create a fork and start developing:

Define a new "domain" for our data types in the phinx.yml configuration

# Domain defined in the config phinx.ml
# This would start a "domain" for our data types
domain:
    phone:    # New user defined data type name
        type: string   # Based on this base phinx data type
        options:       # With these options.
            length: 9
            null: true
    short_status:
        type: enum
        values: pending, started, finished

The intention is to use those as new data types, and they'd just use the values for type and options.

    // Use our user defined types from config
    $table->addColumn("phone_number", "phone");
    $table->addColumn("task_status", "short_status");

Since those are really just aliases for the type and options, we could just add new options to override:

    // Use our user defined types from config
    $table->addColumn("mobile_number", "phone", array("length" => 12, "null" => false));

Let me know what do you think.

[bradynpoulsen]

That would certainly acheive the goals of my example in a much more isolated location instead of having to include a bunch of driver classes

[dereuromark]

Are you able to provide a patch with your suggested changes as PR?

[twoixter]

Hi @dereuromark ! Not yet. It was just a suggestion at the time, but never worked on it.

I'm willing to fork and do a PR if it's still relevant! (This issue dates back to Jan, 2016).

[dereuromark]

You can give it a try and see if that is still relevant. And then a PR would be most welcome!

[twoixter]

Hi @dereuromark ! I just sent PR #1320 for this feature. I'm still fixing some stickler-ci and phpunit version issues for Travis, but you can start looking at it.

[dereuromark]

I tried myself today (as per https://github.com/cakephp/cakephp/pull/11297/files#diff-26ba5244f4728ab88ec0f493f66576e2R37 )

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

and got

InvalidArgumentException: An invalid column type "binaryuuid" was specified for column "uuid".

[dereuromark]

This is now solved for non primary key columns for sure using e.g.

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

For primary key ones we have to further investigate and add test cases.

[geoidesic]

Doesn't seem to work for me. I get InvalidArgumentException: An invalid column type "binaryuuid" was specified for column "id"

robmorgan/phinx                       dev-0.next 7b3ca6c

[dereuromark]

Make sure to use master (as 0.next is behind)
But either way, we need to figure out then why it is throwing this error here.

What DB type are you using? We only enabled this so far for a few DBs.

[geoidesic]

MariaDB (MySQL)

[geoidesic]

Master branch doesn't work for me:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - cakephp/migrations 3.0.0-beta2 requires robmorgan/phinx 0.next-dev -> satisfiable by robmorgan/phinx[dev-0.next].
    - cakephp/migrations 3.0.0-beta2 requires robmorgan/phinx 0.next-dev -> satisfiable by robmorgan/phinx[dev-0.next].
    - cakephp/migrations 3.0.0-beta2 requires robmorgan/phinx 0.next-dev -> satisfiable by robmorgan/phinx[dev-0.next].
    - Can only install one of: robmorgan/phinx[dev-0.next, 0.12.3].
    - Can only install one of: robmorgan/phinx[0.12.3, dev-0.next].
    - Can only install one of: robmorgan/phinx[0.12.3, dev-0.next].
    - Installation request for robmorgan/phinx ^0.12.3 -> satisfiable by robmorgan/phinx[0.12.3].
    - Installation request for cakephp/migrations (locked at 3.0.0-beta2, required as ^3.0) -> satisfiable by cakephp/migrations[3.0.0-beta2].

[dereuromark]

you are using an outdated version here. we have no betas anymore. make sure you upgraded your migrations plugin properly.

[dereuromark]

By the way: This works for me in a migration file for MySQL

		$options = [
			'id' => false,
			'primary_key' => 'id',
		];

		$table = $this->table('uuid_users', $options);
		$table->addColumn('id', 'binaryuuid', [
			'null' => false,
		]);

		$table->addColumn('name', 'string', [
			'default' => null,
			'limit' => 100,
			'null' => false,
		]);

		$table->addColumn('additional_uuid', 'binaryuuid', [
			'default' => null,
			'null' => true,
		]);

		$table->save();

[geoidesic]

I've tested this now on latest Phinx (0.12.3), CakePHP (4.1) and Migrations plugin (3.0.0), on a newly created Cake project. On PHP 7.3.9 and 7.4.1 with MySQL 5.7.26. The migration works but I get this error whenever trying to fill a binaryuuid within a Seed by using Text::uuid.

  SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'alternate_uuid' at row 1

As far as I can tell this means that the uuid is not being converted to binary, although the datatype's code shows that this should happen automatically: https://api.cakephp.org/3.8/source-class-Cake.Database.Type.BinaryUuidType.html#154-167

[geoidesic]

Setting aside the binaryuuid type for a moment, It doesn't seem to me that the toDatabase method of the Type is called. I've tried putting a die statement in there and the seeder carries on merrily regardless, so it's never reached. In fact when working with uuid data types, even if I delete the UuidType.php file from vendor/cakephp/src/Database/Type it still seems to generate my seeded tables.

[geoidesic]

Getting back to binaryuuid it does seem to me that the conversion is not being called, because when I do this, the seeder finally generates a record with (what is presumably) a valid binaryuuid pk:

<?php

use Migrations\AbstractSeed;
use Cake\Utility\Text;

/**
 * Uuid seed.
 */
class AppDataSeeder extends AbstractSeed
{
    private function convert($string)
    {
        $string = str_replace('-', '', $string);
        return pack('H*', $string);
    }

    public function run()
    {
        $data = [
            [
                'id' => $this->convert(Text::uuid()),
                'name' => 'individual',
                'additional_uuid' => Text::uuid(),
            ],
        ];

        $table = $this->table('uuid_users');
        $table->insert($data)->save();
    }
}

[dereuromark]

@lorenzo Isn't the idea of using the ORM wrapper here, that it automatically uses the built in CakePHP type class for this (BinaryUuid)? Which would trigger this conversation?
At least for me in the CakePHP + Migrations plugin context this works as expected.

[geoidesic]

Also ORM validation seems to break. Looks like the default expects id fields to be integers, even when the Type is uuid. [Actually nvm this – Seems to be due to me not having re-baked my Table objects]