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

Baking a fixture with custom data type generates invalid SQL #567

Open
davidyell opened this issue Jul 16, 2019 · 13 comments
Open

Baking a fixture with custom data type generates invalid SQL #567

davidyell opened this issue Jul 16, 2019 · 13 comments
Assignees

Comments

@davidyell
Copy link
Contributor

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.

@markstory markstory added this to the 1.11.x milestone Jul 19, 2019
@markstory
Copy link
Member

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

@davidyell
Copy link
Contributor Author

davidyell commented Jul 19, 2019 via email

@ADmad
Copy link
Member

ADmad commented Jul 19, 2019

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

@davidyell
Copy link
Contributor Author

davidyell commented Jul 19, 2019

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' 🤔

@markstory
Copy link
Member

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 markstory self-assigned this Jul 21, 2019
@chrisch-hh
Copy link

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?

@markstory
Copy link
Member

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 markstory modified the milestones: 1.11.x, 1.12.x Dec 7, 2019
@chrisch-hh
Copy link

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.

@dereuromark
Copy link
Member

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?

@github-actions
Copy link

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 github-actions bot added the stale label Jan 12, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2023
@othercorey othercorey reopened this Jan 28, 2023
@othercorey othercorey added pinned and removed stale labels Jan 28, 2023
@bradmcnaughton
Copy link

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'),
);

@markstory
Copy link
Member

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

@bradmcnaughton
Copy link

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.

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

No branches or pull requests

7 participants