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

Schema generator is not adding COMMENT to custom types #2594

Closed
fkrauthan opened this issue Jan 10, 2017 · 36 comments
Closed

Schema generator is not adding COMMENT to custom types #2594

fkrauthan opened this issue Jan 10, 2017 · 36 comments

Comments

@fkrauthan
Copy link

I am using the DoctrineEnumBundle and have a issue with the enum SQL that gets generated. It does not contain the COMMENT field which is required to identify the enum type correctly.

I've opened a ticket at the DoctrineEnumBundle which contains a lot more information about this case fre5h/DoctrineEnumBundle#87

Information that might be important right of the bet are I am running MariaDB on Windows in Version10.1.20.

Please let me know if you need any other information.

@Ocramius
Copy link
Member

Ocramius commented Jan 11, 2017 via email

@fkrauthan
Copy link
Author

No it does not appear:

> C:\Dev\php\php-7.1\php.exe C:\Users\fkrauthan\PhpstormProjects\play-for-today\flowergame2\bin\console doctrine:schema:create --dump-sql
CREATE TABLE flowers (id INT AUTO_INCREMENT NOT NULL, user_id INT DEFAULT NULL, flower_type_id INT DEFAULT NULL, flower_graphic_set_id INT DEFAULT NULL, last_breed_at DATETIME DEFAULT NULL, state ENUM('seed', 'germ', 'mature') NOT NULL, created_at DATETIME DEFAULT NULL, updated_at DATETIME DEFAULT NULL, INDEX IDX_7DAF2300A76ED395 (user_id), INDEX IDX_7DAF2300818FFC14 (flower_type_id), INDEX IDX_7DAF23004BB6826F (flower_graphic_set_id), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB;

And my custom type looks like this:

<?php
namespace AppBundle\DBAL\Type;

use Fresh\DoctrineEnumBundle\DBAL\Types\AbstractEnumType;

class FlowerStateType extends AbstractEnumType {
	const SEED = 'seed';
	const GERM = 'germ';
	const MATURE = 'mature';

	protected static $choices = [
		self::SEED    => 'Seed',
		self::GERM    => 'Germ',
		self::MATURE    => 'Mature',
	];
}

And I use this state like this in my entity:

/**
	 * @var string
	 *
	 * @ORM\Column(type="FlowerState")
	 *
	 * @DoctrineAssert\Enum(entity="AppBundle\DBAL\Type\FlowerStateType")
	 */
	protected $state;

@Ocramius
Copy link
Member

Wondering if enum is mapped manually in the platform then...

@fkrauthan
Copy link
Author

Any way how I can try debugging this/helping you guys to find the issue? This is really bugging me as it requires all my auto generated migration sqls to be manual adjusted.

@Ocramius
Copy link
Member

@fkrauthan a test case is needed. My suspicion is that this is the culprit.

What you want is two tests:

  • one that checks schema generation without the platform enum type aliased to string
  • one that checks schema generation with the platform enum type aliased to string

Removing that comment and trying your schema generation again should also tell you if that's the culprit, and which part of the code is skipping generating the comment.

As for ENUM in general, it is a bit of a mess: https://stackoverflow.com/questions/8750724/what-do-you-use-instead-of-enum-in-doctrine2

@fkrauthan
Copy link
Author

Hmm how do I change that schema generation checks?

It is just weird coz the enum is recognized as enum. Its just the required NOTE is not added to the enum which results in Doctrine thinking that enum does not exist and the next migration contains again a column change to make it a enum.

@Ocramius
Copy link
Member

Doctrine thinks that your enum is a string because of that line. Comment it out, then retry an orm:schema:create

@fkrauthan
Copy link
Author

Oh you mean when it is reading it from the database? But again the developer of the DoctrineEnumBundle says there should be a database comment added when generating the schema or is this because a enum is a sort of true database feature and not a 100% custom type?

@Ocramius
Copy link
Member

Oh you mean when it is reading it from the database?

Yep

But again the developer of the DoctrineEnumBundle says there should be a database comment added when generating the schema or is this because a enum is a sort of true database feature and not a 100% custom type?

The reason the comments are supposed to be added is that doctrine can't understand a custom type otherwise. For instance, let's say that you have an Image type, that is to be stored as BLOB, doctrine only knows it's a BLOB until it checks also the comment.

I suggest checking that particular line that I referenced, and if that's not it, then a test case doing schema introspection is needed.

@deeky666
Copy link
Member

@fkrauthan you will have to make sure the platform knows that custom requires a comment IIRC:

    Type::addType($name, $class);

    $platform->registerDoctrineTypeMapping($name, $name);
    $platform->markDoctrineTypeCommented($name);

Can you check if that helps?

@deeky666
Copy link
Member

See also: doctrine/DoctrineORMModule#267

@fkrauthan
Copy link
Author

@deeky666 it looks like the https://github.com/fre5h/DoctrineEnumBundle/blob/c73dc724b2d2f18600e1ffb246b017c8f51085ab/DBAL/Types/AbstractEnumType.php file (the base type class) contains a requiresSQLCommentHint which always returns true. Shouldn't that be enough?

Still have to try @Ocramius tests.

@deeky666
Copy link
Member

@fkrauthan unfortunately I really don't know. Have not been around at the time this was implemented. @beberlei do you remember?

@fkrauthan
Copy link
Author

Based on comments in https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/Type.php this is exactly what that method should be used for.

@deeky666
Copy link
Member

@fkrauthan I know it is. But it is not evaluated in AbstractPlatform::registerDoctrineTypeMapping(). Looks like a potential improvement. But I would like to know first if it was implemented like it is by intention. For now the "workaround" (which seems to be an undocumented instruction) should help.

@fkrauthan
Copy link
Author

The problem is I am using Symfony and therefor have no control over executing a custom type registration (I define it in my config file).

@deeky666
Copy link
Member

@fkrauthan if you use Symfony you probably also use DoctrineBundle? Because it seems to be integrated into DoctrineBundle: https://github.com/doctrine/DoctrineBundle/pull/21/files

Still this can be fixed in DBAL transparently.

@fkrauthan
Copy link
Author

Ok that means this will be already marked as commented type as the default value is true and I haven't changed that.

@deeky666
Copy link
Member

@fkrauthan you would have to debug whether your type gets commented by DoctrineBundle then, this is out of scope for us. Meanwhile I will prepare a patch for implicit marking via registerDoctrineTypeMapping().

@deeky666
Copy link
Member

@fkrauthan PR created: #2603

@Ocramius
Copy link
Member

@fkrauthan check if current master helps.

@deeky666
Copy link
Member

Closing this as fixed. If you still have issues with current master feel free to reopen.

@fkrauthan
Copy link
Author

Hey I just updated my doctrine/orm as well as doctrine/dbal to master version but it still generates me migrations without the COMMENT attached to it.

In addition to that I followed @Ocramius recommendation and commented out the line $databasePlatform->registerDoctrineTypeMapping('enum', 'string'); in the FreshDoctrineEnumBundle. The schema:create still works (also without COMMENT attached to it) but if I try to create a migration:diff I get the error:

[Doctrine\DBAL\DBALException]                                                                    
  Unknown database type enum requested, Doctrine\DBAL\Platforms\MySqlPlatform may not support it.

Do you guys have any other ideas where I could start debugging why the COMMENT is not attached even so my enum is a Custom Type within my code?

@Ocramius
Copy link
Member

@fkrauthan does the exception occur when running orm:create-schema on a clean schema?

@deeky666
Copy link
Member

@fkrauthan please try to isolate the issue to DBAL code only, otherwise it's hard for us to get into the problem. I suggest you try to create a DBAL test case and see if you can get the issue reproduced. To me it still looks like some misconfiguration or bug in the bundle(s) you use, as we have a lot of successful tests around that kind of thing.

@fkrauthan
Copy link
Author

That is not so easy as I am running into that issue in combination with Symfony and a Symfony Bundle that extends DBAL. Would it help if I set up a mini symfony installation just using that bundles related to the enum for testing?

@Ocramius
Copy link
Member

Would it help if I set up a mini symfony installation

Unlikely, since we never get the time to check full stacks. The issue needs to be reproduced within our suite, without symfony interaction...

@deeky666
Copy link
Member

deeky666 commented Jan 27, 2017

@fkrauthan you could try debugging your setup and check if the custom type is registered as commented type in the DBAL platform. Should not be so hard. Just put in some var_dump() at certain locations in the DBAL platform and analyze.

@fkrauthan
Copy link
Author

Sure. I am not very familiar with dbal so if you could pin point me to a couple of locations where I should place some var_dumps and what I should see that would help me a lot.

@deeky666
Copy link
Member

@fkrauthan the interesting parts are here and here

@fkrauthan
Copy link
Author

Ok registerDoctrineTypeMapping is called with the parameters $dbType=enum and $doctrineType=string

the second method markDoctrineTypeCommented is never called.

@fkrauthan
Copy link
Author

I might have found something in the DoctrineBundle code. The ConnectionFactory.php#L60 registers custom types (and detects correctly that my custom types should be commented/adding it to the $commentedTypes variable). But when createConnection is called $mappingTypes is an empty array and therefor the for loop that looks thru all commentedTypes and calls markDoctrineTypeCommented is never executed. For testing purpose I moved that for loop outside of the if condition and now my migration contains correctly the comment.

I guess I should open a ticket at the DoctrineBundle github repo? The only thing that is still a bit weird to me is why this is even required? The Doctrine\DBAL\Types\Type class contains requiresSQLCommentHint. Shouldn't this always win and add a comment regardless if you explicit call markDoctrineTypeCommented?

@deeky666
Copy link
Member

@fkrauthan we did something about this in DBAL, but currently only available in master. #2603

@fkrauthan
Copy link
Author

Is there already a timeline to release that as a new version?

@Ocramius
Copy link
Member

@fkrauthan nope

@Ocramius Ocramius changed the title Schema generator is not adding COMMENT to custom type Schema generator is not adding COMMENT to custom types Jul 22, 2017
f3l1x referenced this issue in contributte/doctrine-dbal Apr 4, 2018
@github-actions
Copy link

github-actions bot commented Aug 4, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants