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

Invalid ALTER TABLE statement for Postgresql after enum update #94

Open
el-seirh opened this issue Apr 12, 2017 · 4 comments
Open

Invalid ALTER TABLE statement for Postgresql after enum update #94

el-seirh opened this issue Apr 12, 2017 · 4 comments
Assignees
Labels

Comments

@el-seirh
Copy link

Hi there

I have tested this bundle with an installation of a Postgresql 9.6 (current) database.
In the \Fresh\DoctrineEnumBundle\DBAL\Types\AbstractEnumType::getSqlDeclaration() function the bundle has a special return format for Postgresql:

        if ($platform instanceof PostgreSqlPlatform || $platform instanceof SQLServerPlatform) {
            return sprintf('VARCHAR(255) CHECK(%s IN (%s))', $fieldDeclaration['name'], $values);
        } 

While this works perfectly for CREATE TABLE, it creates an invalid query for ALTER TABLE:
ALTER TABLE postal_address ALTER address_type TYPE VARCHAR(255) CHECK(address_type IN ('private', 'company', 'organisation'));

If at the moment I'd know how to make a valid ALTER TABLE statement using a check constraint I'd gladly provide you with a pull request. But I don't ;(.

If I had to do it manually I would drop the constraint and recreate it but I see that is not an easy thing to to in this context.
At the moment the migration works only once and will never work again when a EnumType changes.

Really cool would be real enum support (which is technically possible in Postgres) but neither do I know how to create an enum inside a CREATE TABLE statement. I don't really think that's possible. You'd need CREATE TYPE.

Regards

@fre5h fre5h self-assigned this Apr 12, 2017
@fre5h fre5h added the question label Apr 12, 2017
@fre5h
Copy link
Owner

fre5h commented Apr 12, 2017

@el-seirh Please check this comment #31 (comment)
The key point is bundle cannot create two different SQL statements which are related to one field. Only one definition for field is allowed.

@el-seirh
Copy link
Author

el-seirh commented Apr 13, 2017

That is why only field definition syntax is allowed ALTER TABLE table ALTER field TYPE VARCHAR(255) CHECK(field IN ('value', 'value', 'value'))

That's exactly what I assumed. So ok... only field definition syntax is allowed. But then you still have the problem, that the ALTER TABLE statement is simply invalid. It cannot work. Even if you paste this directly into the database it will not work. Even when the previous check constraint was removed.

What I'm trying to say here is that your mentioned Hook does not work for posgtgres.

So ... we have to agree that it's simply not possible to create a valid field definition statement for postgres once the pseudo-enum has been created?

@liogate
Copy link

liogate commented Jun 25, 2018

Hi, this subject is old but I found an alternative to update enum in postgres using migration process.

ALTER TABLE table DROP CONSTRAINT table_column_check;
ALTER TABLE table ADD CONSTRAINT table_column_check CHECK(category IN ('value1','value2'));

Hope this can be helpful.

@flaushi
Copy link

flaushi commented Aug 10, 2018

so in a migration, one could include

$values = \implode(
            ', ',
            \array_map(
                function ($value) {
                    return "'{$value}'";
                },
                YourEnumType::getValues()
                )
            );
$this->addSql('ALTER TABLE table DROP CONSTRAINT table_column_check');
$this->addSql('ALTER TABLE table ADD CONSTRAINT table_column_check CHECK(category IN ($values))');

Would it be possible to do this automatically when a migration is created (via diff)?

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

No branches or pull requests

4 participants