sqlalchemy-bot / test_alembic_1

0 stars 0 forks source link

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

Open sqlalchemy-bot opened 8 years ago

sqlalchemy-bot commented 8 years ago

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 commented 8 years ago

Michael Bayer (zzzeek) wrote:

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

sqlalchemy-bot commented 8 years ago

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 commented 8 years ago

d9k () wrote:

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

sqlalchemy-bot commented 7 years ago

Mitchell Grice () wrote:

@zzzeek link is now broken

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

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

sqlalchemy-bot commented 7 years ago

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 commented 7 years ago

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:

#!python

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:

#!sql

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 commented 7 years ago

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 commented 7 years ago

Michael Bayer (zzzeek) wrote:

great. Do you have a pull request?

sqlalchemy-bot commented 7 years ago

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 commented 7 years ago

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 commented 8 years ago

Changes by d9k ():

sqlalchemy-bot commented 8 years ago

Changes by d9k ():

sqlalchemy-bot commented 8 years ago

Changes by d9k ():

sqlalchemy-bot commented 8 years ago

Changes by d9k ():

sqlalchemy-bot commented 8 years ago

Changes by d9k ():

sqlalchemy-bot commented 8 years ago

Changes by d9k ():

sqlalchemy-bot commented 8 years ago

Changes by Michael Bayer (zzzeek):