alembic_utils icon indicating copy to clipboard operation
alembic_utils copied to clipboard

gracefully handle when a migrator needs to modify a column that a view depends on

Open nstrong-scw opened this issue 2 years ago • 8 comments

Alembic version: 1.7.7 Alembic_utils version: 0.7.7 db: postgres 12

Here's a basic setup:

-- pretend I have SQLAlchemy classes set up to create these
CREATE TABLE A (
  id integer not null primary_key,
  some_field varchar(255)
);

CREATE VIEW A_VIEW(id, some_field) AS
  SELECT id, some_field FROM A;

Now, suppose you wish to change some_field to a text column. I change the class, run alembic revision -m 'update table' --autogenerate.

Expected behavior: the column should upgrade successfully. Actual behavior: Postgres throws an error: "cannot alter type of a column used by a view or rule"

So, in other words, alembic_utils will successfully recreate views if the view definition is changed, but it doesn't catch when table columns associated with those views get modified.

I've devised a workaround, but it would be nice if it could be done automatically somehow.

Here is my (admittedly naive) workaround to solve this problem in the current code:

# - dependent_view_names is a list of view names that my migration conflicts with
# - lambda_func: a function that does the actual migration steps
def _wrap(dependent_view_names, lambda_func):
    # start by retrieving the views that currently exist at migration time
    existing_views = {v.signature: v for v in PGView.from_database(op.get_bind(), 'public')}
    for view in dependent_view_names:
        if view in existing_views:
            # drop the view and any dependent views
            op.drop_entity(existing_views[view], cascade=True)
    # do the thing
    lambda_func()
    # we replace all views, because the cascade delete may have deleted multiple views.
    for view in existing_views:
        op.replace_entity(existing_views[view])

def upgrade():
    _wrap(['A_VIEW'], op.alter_column('A', 'some_field', existing_type=sa.VARCHAR(length=255), type_=sa.Text(), existing_nullable=True))

def downgrade():
    _wrap(['A_VIEW'], op.alter_column('A', 'some_field', existing_type=sa.TEXT(), type_=sa.VARCHAR(length=255), existing_nullable=True))

nstrong-scw avatar May 05 '22 19:05 nstrong-scw

There is an experimental context manager for this use case:

from alembic_utils.depends import recreate_dropped

     def upgrade() -> None: 
  
         my_view = PGView(...) 
  
         with recreate_dropped(op.get_bind()) as conn: 
  
             op.drop_entity(my_view) 
             # you could also do `op.execute("drop view myview")` here
  
             # change an integer column to a bigint 
             op.alter_column( 
                 table_name="account", 
                 column_name="id", 
                 schema="public" 
                 type_=sa.BIGINT() 
                 existing_type=sa.Integer(), 
             ) 

when the context manager goes out of scope, anything you drop gets recreated

Please note that it is not currently part of the public API so it is use-at-your-own-risk

olirice avatar May 05 '22 21:05 olirice

Thanks for this!

I looked over the code behind recreate_dropped and it looks good to me. It's certainly a nicer way to handle the issue, although it'd still be cool if the need for it could be auto-detected at generate time.

The logging tends to be a little chatty (the "Resolving entities with dependencies" message gets repeated across each round), but that's a very minor complaint.

nstrong-scw avatar May 05 '22 21:05 nstrong-scw

it'd still be cool if the need for it could be auto-detected at generate time.

the tricky part w/ that is that the alembic migrations need to be executed for the schema diffing alembic_utils uses to work. That would be fine 80% of the time but some failure cases are:

  • if the alembic op can't be executed inside a transaction ( e.g. create index concurrently ) -> exception
  • if the operation op is slow (populating table / creating an index synchronously) then it will take a long time and might lock up the database

olirice avatar May 05 '22 22:05 olirice

The logging tends to be a little chatty (the "Resolving entities with dependencies" message gets repeated across each round), but that's a very minor complaint

If you'd like to open a PR I'm happy for that log line to more outside the loop

https://github.com/olirice/alembic_utils/blob/5f670039a9a10722c0e5522370d8591831855e54/src/alembic_utils/depends.py#L34

olirice avatar May 05 '22 22:05 olirice

I took a stab at optimizing solve_resolution_order little bit:

https://github.com/olirice/alembic_utils/pull/90

I'm getting an error KeyError: 'formatters' when I try to run tests. The error is coming from trying to parse the config file (alembic.ini), but I'm not sure why it can't find it. So, I don't know if the tests still pass or not--I'd appreciate guidance.

nstrong-scw avatar May 06 '22 01:05 nstrong-scw

Here are the steps to run the tests

Requirements:

  • docker-compose
  • python
git clone [email protected]:olirice/alembic_utils.git
cd alembic_utils
python -m venv venv
source venv/bin/activate
pip install -U pip
pip install -e ".[dev]"
pytest

If that fails, please add a comment with:

  • your OS
  • python version
  • a complete stacktrace
  • output of pip freeze

so i can try to reproduce

olirice avatar May 06 '22 14:05 olirice

No dice.

  • OS: MacOS 12.3.1
  • Python version: 3.10.3

pytest.log pip.freeze.txt

As a side note, I'm not sure if it is relevant to the tests failing, but I had to re-pin pypy to the latest version because the old version pulls in a version of check-ast that doesn't build against python 3.9 and later.

nstrong-scw avatar May 06 '22 15:05 nstrong-scw

unfortunately i can not reproduce that error with 3.10.3 and the lib versions from pip freeze

It looks like its failing to find this key https://github.com/olirice/alembic_utils/blob/master/alembic.ini#L59 in this function https://github.com/olirice/alembic_utils/blob/5f670039a9a10722c0e5522370d8591831855e54/src/alembic_utils/testbase.py#L36

if you track it down I'd be happy to fix. In the meantime, please feel free to open PRs and let CI run the test suite for you as needed (mark as draft)

thanks

olirice avatar May 06 '22 17:05 olirice