bake icon indicating copy to clipboard operation
bake copied to clipboard

Baking a fixture with custom data type generates invalid SQL

Open davidyell opened this issue 6 years ago • 13 comments

Environment

  • CakePHP 3.7.9
  • Bake 1.9.6
  • PHP 7.3.6

What I did

bin/cake bake fixture Calls

This generated my fixture file as normal. Then I implemented this into my integration test using public $fixtures = ['app.Calls'].

I have a custom data type set to a field in this table, configured in the Table class.

What happened

My test failed because the SQL generated from the fixture was invalid. This was because the fixture had created a schema setup of the following. 'telephone_number' => ['type' => 'telephone', 'length' => 255, 'null' => false, 'default' => null, 'collate' => 'utf8mb4_general_ci', 'comment' => '', 'precision' => null, 'fixed' => null],

This obviously fails as telephone is not a valid data type for MySQL.

davidyell avatar Jul 16 '19 09:07 davidyell

How should bake find out the column type when the model has mapped the column to a custom type?

markstory avatar Jul 19 '19 02:07 markstory

I would assume a string. As it'll store almost any data type.

On Fri, 19 Jul 2019, 03:35 Mark Story, [email protected] wrote:

How should bake find out the column type when the model has mapped the column to a custom type?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cakephp/bake/issues/567?email_source=notifications&email_token=AAAMFYOKLWZWYULF6CGNIFTQAESATA5CNFSM4ID7COF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2KMPZQ#issuecomment-513066982, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAMFYNML6NN6J2RU6NAT33QAESATANCNFSM4ID7COFQ .

davidyell avatar Jul 19 '19 07:07 davidyell

TypeInterface does have getBaseType() method. Could we leverage that (assuming dev has setup proper base type)?

ADmad avatar Jul 19 '19 08:07 ADmad

I like that solution @ADmad could we enforce that method in userland code somehow? I notice it's in the interface, and implemented into Type.

Would it be worth creating an AbstractClass for userland types to inherit from to ensure the method exists? Or is that almost too much change to the core for what is essentially an edge-case?

Perhaps better served as a documentation change to the 'Creating your own types' 🤔

davidyell avatar Jul 19 '19 09:07 davidyell

Updating the docs for custom types and having fixtures use it when creating schema sounds like a good solution. Custom types would still be included in fixture $fields and schema generation would be dependent on the base type defined in the type class.

markstory avatar Jul 21 '19 16:07 markstory

Any news about this?

I'm using a custom data type OrderedUuid which extends BinaryUuid. Baking the fixtures gives me:

public $fields = [
 'uuid' => ['type' => 'ordereduuid', 'length' => null, 'null' => false, 'default' => null, 'comment' => '', 'precision' => null]
...
]

Running tests isn't possible due to SQL error. The generated SQL for table creation looks like this:

CREATE TABLE `myTable` (
 `uuid` NOT NULL
...
)

Obviously the data type is missing in this statement. It should be BINARY(16) in my case.

Is there a way to define the data type?

chrisch-hh avatar Dec 03 '19 14:12 chrisch-hh

Is there a way to define the data type?

Not right now there isn't which is the source of this problem. Type mappers don't know and can't reasonably know about their storage data type as storage types are dialect specific. The schema dialects that convert abstract types into dialect specific ones are not aware of custom types. We might be able to solve this by adding an interface that allows type classes to define a 'parent's abstract type that they use for storage.

markstory avatar Dec 04 '19 14:12 markstory

For those who might be interested in a workaround to use custom data types with bake fixtures:

Add your custom data type (e.g. mycustomtype which should be a BINARY(16)) in cake/vendor/cakephp/cakephp/src/Database/Schema/TableSchemaInterface.php:

/**
 * My custom data type
 *
 * @var string
 */
const TYPE_MY_CUSTOM_TYPE = 'mycustomtype';

If you use MySQL, add in cake/vendor/cakephp/cakephp/src/Database/Schema/MysqlSchema.php into the function columnSql():

...
TableSchema::TYPE_BIGINTEGER => ' BIGINT',
TableSchema::TYPE_BINARY_UUID => ' BINARY(16)',

TableSchema::TYPE_MY_CUSTOM_TYPE => ' BINARY(16)', // use mysql type BINARY(16)

TableSchema::TYPE_BOOLEAN => ' BOOLEAN',
TableSchema::TYPE_FLOAT => ' FLOAT',
TableSchema::TYPE_DECIMAL => ' DECIMAL',
...

If you use another database then edit the corresponding schema file. With these changes, bake will generate valid SQL for fixture creation.

Be careful: those changes will get lost if you update CakePHP.

Still hoping that there will be an option in future CakePHP releases to define the database type in the custom data type.

chrisch-hh avatar Jan 14 '20 08:01 chrisch-hh

We could try to look up based on the class, and otherwise just default to text or some default that will always work? Better than failure maybe?

dereuromark avatar Mar 16 '20 22:03 dereuromark

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days

github-actions[bot] avatar Jan 12 '23 00:01 github-actions[bot]

For anyone trying @chrisch-hh 's solution for POINT or other geometry (with mysql) such as the one described in the cookbook (https://book.cakephp.org/4/en/orm/database-basics.html#mapping-custom-datatypes-to-sql-expressions)

Make the following changes:

Add to TableSchemaInterface.php

/**
 * Point data type
 *
 * @var string
 */
const TYPE_POINT = 'point';

Add to typeMap in MysqlSchemaDialect.php (note MysqlSchema deprecated and replaced with this in v 4.1+)

...
TableSchema::TYPE_POINT => ' POINT',
...

In your fixture file define the POINT value as a string containing the coordinates separated by a space. E.G. in LocationsFixture.php inside init()

$this->records = array(
        array('id' => '13683', 'locality' => 'ADELAIDE', 'coordinates' => '-34.932829,138.603813'),
);

bradmcnaughton avatar Feb 20 '23 12:02 bradmcnaughton

@bradmcnaughton You shouldn't need to patch cakephp to add custom types. There are public APIs for all of that.

markstory avatar Feb 28 '23 04:02 markstory

Thanks @markstory, you're correct it looks like the issue I was having was just in the format of the coordinates/point data in the fixture file.

bradmcnaughton avatar Mar 05 '23 04:03 bradmcnaughton