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

DBAL-1193: "requiresSQLCommentHint" is ignored by migration if the SQL type does not change #1137

Closed
doctrinebot opened this issue Apr 8, 2015 · 4 comments
Assignees
Labels

Comments

@doctrinebot
Copy link

Jira issue originally created by user vbence:

I have found a previous issue, http://www.doctrine-project.org/jira/browse/[DBAL-1085](http://www.doctrine-project.org/jira/browse/DBAL-1085) which tells about the need to override "requiresSQLCommentHint" if the custom type uses an SQL type which is already known by Doctrine.

See also:

}
/**
* If this Doctrine Type maps to an already mapped database type,
* reverse schema engineering can't take them apart. You need to mark
* one of those types as commented, which will have Doctrine use an SQL
* comment to typehint the actual Doctrine Type.
*
* @param \Doctrine\DBAL\Platforms\AbstractPlatform $platform
*
* @return boolean
*/
public function requiresSQLCommentHint(AbstractPlatform $platform)
{

The bug: After I learned this and added the required TRUE return value, the migration (generated by "diff") won't contain the comment.

After adding the comment manually, everything works as expected.

@cleentfaar
Copy link

I can confirm this is an issue. Changing a field's type from integer to money (custom type that extends integer, and returns true for requiresSQLCommentHint) should have created a comment on doctrine:schema:update --dump-sql, but it doesn't:

current output:

ALTER TABLE foobar ALTER fruit TYPE INT
ALTER TABLE foobar ALTER fruit DROP DEFAULT

expected output:

ALTER TABLE foobar ALTER fruit TYPE INT
ALTER TABLE foobar ALTER fruit DROP DEFAULT
COMMENT ON COLUMN foobar.fruit IS '(DC2Type:money)'

Oh and we are using postgresql, but I'm pretty sure it affects other drivers too. Hope this helps!

@brud
Copy link

brud commented Jun 28, 2016

Hi, I have similar issue. I add CITEXT type mapping (on PostgreSQL), and in each migration I have:

//migration
$this->addSql('ALTER TABLE managers ALTER email TYPE CITEXT');
$this->addSql('ALTER TABLE managers ALTER email DROP DEFAULT');
$this->addSql('ALTER TABLE managers ALTER email TYPE CITEXT');

//CitextType
public function requiresSQLCommentHint(AbstractPlatform $platform)
    {
        return $platform->getName() !== 'postgresql'; // I also tries to return true - no effect
    }

Even if previously successfully migration contain the same queries.

@morozov
Copy link
Member

morozov commented Aug 5, 2022

Irrelevant as of #5512.

@morozov morozov closed this as not planned Won't fix, can't repro, duplicate, stale Aug 5, 2022
@github-actions
Copy link

github-actions bot commented Sep 5, 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 Sep 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants