alembic icon indicating copy to clipboard operation
alembic copied to clipboard

detect check constraint changes applied to schema types esp non native ENUM

Open sqlalchemy-bot opened this issue 9 years ago • 30 comments

Migrated issue, originally created by d9k

Hi! I use alembic 0.8.5 (tried at 0.8.0 at first, then updated), postgresql 9.3, ubuntu 14.04

I have migration code

op.add_column('my_table', sa.Column('status',
  sa.Enum('one', 'two', 'three', name='test_enum', native_enum=False), nullable=True
))

after migration apply and database dump I see

CREATE TABLE my_table (
-- . . . . .
    status character varying(5),
    CONSTRAINT test_enum CHECK (((status)::text = ANY ((ARRAY['one'::character varying, 'two'::character varying, 'three'::character varying])::text[])))
);

After this I try to alter enum typed field with this new autogenerated from model migration:

op.alter_column('my_table', 'status',
   existing_type=sa.VARCHAR(length=5),
   type_=sa.Enum('one', 'two', 'three', 'four', name='test_enum', native_enum=False),
   existing_nullable=True)

And I get error on migration apply:

sqlalchemy.exc.ProgrammingError: (psycopg2.ProgrammingError) constraint "test_enum" for relation "my_table" already exists

I suggest that old constraint (if exists) must be removed prior to new constraint creation. It must be considered that old type for non-native enum is VARCHAR. As in my previous issue https://bitbucket.org/zzzeek/alembic/issues/362/postgresql-migration-float-precision-fail it looks like alembic developers didn't make alembic autogeneration script aware of alembic's own hacks for different RDBMS'es which it uses to alter/create a schema. Why?

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

related to #180, relies upon https://bitbucket.org/zzzeek/sqlalchemy/issues/1443/reflect-check-constraints-uniques-done

sqlalchemy-bot avatar Mar 17 '16 15:03 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

it looks like alembic developers didn't make alembic autogeneration script aware of alembic's own hacks for different RDBMS'es which it uses to alter/create a schema. Why?

Please read the documentation at http://alembic.zzzcomputing.com/en/latest/autogenerate.html#what-does-autogenerate-detect-and-what-does-it-not-detect which answers this question exactly (and also suggests that we might not want to attempt making this work).

sqlalchemy-bot avatar Mar 17 '16 15:03 sqlalchemy-bot

d9k wrote:

Thank you for answer and link to other issue and documentation, @zzzeek!

sqlalchemy-bot avatar Mar 17 '16 15:03 sqlalchemy-bot

Mitchell Grice wrote:

@zzzeek link is now broken

sqlalchemy-bot avatar Mar 31 '17 07:03 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

http://alembic.zzzcomputing.com/en/latest/autogenerate.html#what-does-autogenerate-detect-and-what-does-it-not-detect

sqlalchemy-bot avatar Mar 31 '17 14:03 sqlalchemy-bot

ProgBot (@progbot) wrote:

Quick question - I've seen other issues marked as wontfix for detecting Enum changes (#270, #329). Did this view change?

I don't completely understand the reasoning there - "the representation of such a type in the non-supporting database, i.e. a CHAR+ CHECK constraint, could be any kind of CHAR+CHECK. For SQLAlchemy to determine that this is actually an ENUM would only be a guess". Is this saying that there might be multiple CHAR+CHECK constraints, so there's no way to determine which needs to be changed? I ask because defining the column as an Enum within SQLAlchemy should be enough to know that the column IS an enum (I'd think)

sqlalchemy-bot avatar Aug 08 '17 15:08 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

I've seen other issues marked as wontfix for detecting Enum changes (#270, #329). Did this view change?

those are two very different issues. #270 is no bug. In #329, you would now implement a compare_type handler that does what you need.

The reason this is extremely difficult to build into alembic is because only MySQL has a simple "ENUIM" type that runs inline in the table; no other database has this. Postgresql's version is a much more awkward CREATE TYPE ordeal where the type itself can even be shared among multiple tables; to hide this transparently would be a lot of effort. Just getting the CREATE TYPE/DROP TYPE aspect of this to work in all cases within SQLAlchemy has taken years to get somewhat right. Then, users using the magic "detect ENUM change" feature would then see the feature fail on all other databases as well as when native_enum is false on MySQL/PG, so for all that effort it still doesn't really work. I can guarantee you that if I spent the many weeks to make all this "work", I'd have just as many bug reports about all the edge cases not working as I do now about it not being available at all, and now they'd be much more urgent. I'm just one person, the ratio of feasibility to effort is not hitting it for this one.

I don't completely understand the reasoning there - "the representation of such a type in the non-supporting database, i.e. a CHAR+ CHECK constraint, could be any kind of CHAR+CHECK. For SQLAlchemy to determine that this is actually an ENUM would only be a guess". Is this saying that there might be multiple CHAR+CHECK constraints, so there's no way to determine which needs to be changed?

it means that Alembic cannot tell if the given column is intended to be an ENUM:

Table(
    't1', m, Column("data", String(50)),
    CheckConstraint("data IN ('foo', 'bar')")
)

Table(
    't2', m, Column("data", Enum("a", "b", "c"))
)

because here is the CREATE TABLE:

CREATE TABLE t2 (
	data VARCHAR(1), 
	CHECK (data IN ('a', 'b', 'c'))
)


CREATE TABLE t1 (
	data VARCHAR(50), 
	CHECK (data IN ('foo', 'bar'))
)

which one is the "ENUM" ?

sqlalchemy-bot avatar Aug 08 '17 16:08 sqlalchemy-bot

ProgBot (@progbot) wrote:

Why can't you use target_metadata.tables to determine which column is an Enum? Even if the type is changed, you should know the original via the migrations, and the current through target_metadata

sqlalchemy-bot avatar Aug 08 '17 16:08 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

great. Do you have a pull request?

sqlalchemy-bot avatar Aug 08 '17 16:08 sqlalchemy-bot

ProgBot (@progbot) wrote:

Not yet :) Frankly I was just curious what is possible. I may have time to dig into the autogenerate code in a couple weeks. I am curious if there is a reason why my approach wouldn't work - I am looking at this naively, not having experience with the codebase.

sqlalchemy-bot avatar Aug 08 '17 16:08 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

if you have Enum on the Python metadata side, and then you have a CHECK constraint on the database side, then sure, you can try comparing in that case (hence this issue is still opened). An additional issue is that you need to parse the elements in the CHECK constraint also which in particular is going to be difficult if the values themselves have quoting issues.

But most people are suffering on the Postgresql side with the CREATE TYPE thing so I'd really need all of that addressed.

sqlalchemy-bot avatar Aug 08 '17 17:08 sqlalchemy-bot

Changes by d9k:

  • edited description

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Changes by d9k:

  • edited description

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Changes by d9k:

  • edited description

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Changes by d9k:

  • edited description

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Changes by d9k:

  • edited description

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Changes by d9k:

  • changed title from "Postgresql migration: enum alter fail" to "PostgreSql migration: non-native enum alter fail"

sqlalchemy-bot avatar Mar 17 '16 04:03 sqlalchemy-bot

Changes by Michael Bayer (@zzzeek):

  • removed labels: bug
  • added labels: feature

sqlalchemy-bot avatar Mar 17 '16 15:03 sqlalchemy-bot

Changes by Michael Bayer (@zzzeek):

  • added labels: autogenerate - detection

sqlalchemy-bot avatar Mar 17 '16 15:03 sqlalchemy-bot

Changes by Michael Bayer (@zzzeek):

  • changed title from "PostgreSql migration: non-native enum alter fail" to "detect check constraint changes applied to schema "

sqlalchemy-bot avatar Mar 17 '16 15:03 sqlalchemy-bot

Facing the same issue. I have an enum column in one of the model and when I have added few more values and migrate later then it's not being identified as a change. For now, what I did was I went through the migration files and changed it accordingly.

dinukadesilva avatar Sep 17 '19 06:09 dinukadesilva

Facing the same issue. I have an enum column in one of the model and when I have added few more values and migrate later then it's not being identified as a change. For now, what I did was I went through the migration files and changed it accordingly.

what kind of ENUM? The theme here is that there are at least three different types of ENUM your database may have implemented.

zzzeek avatar Sep 17 '19 13:09 zzzeek

I'm having the same issue with a postgres native enum:

class WorkEventType(str, Enum):
    Start = 'start'
    Finish = 'finish'

class WorkEvent(db.Model):
    type = db.Column(db.Enum(WorkEventType))
    ...

My enum was autogenerated by alembic (via flask-migrate) when I created it, but when I add more values to WorkEventType, they go unnoticed. Any advice?

Thanks, Oliver

OliverEvans96 avatar Apr 16 '20 21:04 OliverEvans96

autogenerate does not do PG ENUM at all, you have to enter these into your migrantion file manually. Also, Posgresql likely requires that you suspend the transcation in order to add the enumerated types so you will have to use the autocommit_block method, which includes an example: https://alembic.sqlalchemy.org/en/latest/api/runtime.html?highlight=autocommit#alembic.runtime.migration.MigrationContext.autocommit_block

overall, PG ENUMs are not very flexible or developer friendly.

zzzeek avatar Apr 17 '20 00:04 zzzeek

+1 this issue. Support for detecting changes to a native_enum=False "enum" (check constraint backend) would be very useful.

Has anyone been able to workaround altering the check constraint reliably? I am considering dropping the constraint, and recreating the constraint myself manually. My concern is that this won't generate in the same way that SQLAlchemy would expect, and could cause some hidden problems. Do you anticipate this approach would cause issues @zzzeek ?

FWIW my backend is PG.

henrycjc avatar Nov 26 '20 01:11 henrycjc

+1 this issue. Support for detecting changes to a native_enum=False "enum" (check constraint backend) would be very useful.

please note that SQLAlchemy has simply changed the default behavior of Enum to no longer create this constraint in version 1.4. you can set create_constraint=True to recreate it but longer term I expect very few people will want to use this flag as being able to maintain the CHECK constraint is very tedious. it's a feature I should never have added.

Has anyone been able to workaround altering the check constraint reliably? I am considering dropping the constraint, and recreating the constraint myself manually.

That's kind of your only option right now.

My concern is that this won't generate in the same way that SQLAlchemy would expect, and could cause some hidden problems. Do you anticipate this approach would cause issues @zzzeek ?

none whatsoever. SQLAlchemy only knows how to emit CREATE / DROP and doesn't do anything with constraints at runtime.

zzzeek avatar Nov 26 '20 17:11 zzzeek

Has anyone been able to workaround altering the check constraint reliably? I am considering dropping the constraint, and recreating the constraint myself manually

That's kind of your only option right now.

Another way is to alter the type of the column:

op.alter_column(
        ...
        existing_type=sa.Enum(
            'ONE',
            'TWO',
            native_enum=False,
            ...
        ),
        type_=sa.Enum(
            'ONE',
            'TWO',
            'THREE',
            native_enum=False,
            ...
        ),

dactrungnguyen avatar Apr 06 '21 16:04 dactrungnguyen

Just to add a last comment. Using existing_type and type_ was not working for me. And with native_enum=False, when I try to update it just considers a simple VARCHAR, so I wasn't sure about it performing check. This is a migration in which I alter the enum. I have to perform it manually. ✅ If we try to remove a value used, it will throw an error. ❌ However, if we delete the column and type is no longer used, it is not deleted. We have to delete it manually.

"""empty message

Revision ID: 8fc9087177f9
Revises: c4831cddfd46
Create Date: 2024-02-04 15:41:59.574439

"""
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa
import sqlalchemy_utils


# revision identifiers, used by Alembic.
revision: str = '8fc9087177f9'
down_revision: Union[str, None] = 'c4831cddfd46'
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
    tables = ["carriers"]
    enum = "carriertypeenum"
    tmp_enum = f"{enum}_tmp"
    values = ("CTT", "CORREOS", "GLS", "SEUR", "WITHOUT_AGENCY")

    op.execute(f"CREATE TYPE {tmp_enum} AS ENUM {values}")
    for table in tables:
        op.execute(f"ALTER TABLE {table} ALTER COLUMN type TYPE {tmp_enum} USING type::text::{tmp_enum}")
    op.execute(f"DROP TYPE {enum}")
    op.execute(f"ALTER TYPE {tmp_enum} RENAME TO {enum}")


def downgrade() -> None:
    tables = ["carriers"]
    enum = "carriertypeenum"
    tmp_enum = f"{enum}_tmp"
    values = ("CORREOS", "GLS", "SEUR", "WITHOUT_AGENCY")

    op.execute(f"CREATE TYPE {tmp_enum} AS ENUM {values}")
    for table in tables:
        op.execute(f"ALTER TABLE {table} ALTER COLUMN type TYPE {tmp_enum} USING type::text::{tmp_enum}")
    op.execute(f"DROP TYPE {enum}")
    op.execute(f"ALTER TYPE {tmp_enum} RENAME TO {enum}")

dajimenezriv avatar Feb 04 '24 16:02 dajimenezriv