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

Can't create trigger on the table from the same migration #79

Open
LKay opened this issue Jan 28, 2022 · 9 comments
Open

Can't create trigger on the table from the same migration #79

LKay opened this issue Jan 28, 2022 · 9 comments

Comments

@LKay
Copy link

LKay commented Jan 28, 2022

As title suggest, these seems to be a bug when creating trigger on a table that doesn't exist yet and is part of the same migration. In such case autogenerated create fails with error.

Example definition:

schema = 'public'

extension_uuid = PGExtension(
    schema=schema,
    signature='uuid-ossp',
)

function_updated_at_auto = PGFunction(
    schema=schema,
    signature="updated_at_auto()",
    definition="""
        RETURNS TRIGGER LANGUAGE plpgsql AS $$
        BEGIN
          NEW.updated_at = NOW();
          RETURN NEW;
        END;
        $$;
    """
)

test = sa.Table(
    "test",
    metadata,
    sa.Column("id", pg.UUID, nullable=False, server_default=sa.func.uuid_generate_v1mc(), primary_key=True),
    sa.Column("name", pg.TEXT, nullable=False),
    sa.Column("created_dt", pg.TIMESTAMP(timezone=True), nullable=False, server_default=sa.func.now()),
    sa.Column("updated_dt", pg.TIMESTAMP(timezone=True), nullable=False, server_default=sa.func.now()),
)

trigger_test = PGTrigger(
    schema=schema,
    signature=f"{terst.name}_updated_at_trigger",
    on_entity=f"{schema}.{test.name}",
    definition=f"""
        BEFORE UPDATE ON {schema}.{test.name}
        FOR EACH ROW EXECUTE PROCEDURE {schema}.updated_at_auto()
    """,
)

And registering this in env.py:

register_entities([
    extension_uuid,
    function_updated_at_auto,
    trigger_test
])

This results in error:

INFO  [alembic_utils.replaceable_entity] Detecting required migration op PGTrigger PGTrigger: public.test_updated_at_trigger False public.test
...
sqlalchemy.exc.ProgrammingError: (sqlalchemy.dialects.postgresql.asyncpg.ProgrammingError) <class 'asyncpg.exceptions.UndefinedTableError'>: relation "public.test" does not exist
[SQL: CREATE TRIGGER "test_updated_at_trigger" BEFORE UPDATE ON public.test FOR EACH ROW EXECUTE PROCEDURE public.updated_at_auto()]
(Background on this error at: https://sqlalche.me/e/14/f405)

Creating trigger works, however if table already exists. As a workaround the migration has to be broken up into two, first with all entities and second with only triggers attached to the tables. This seems like a bug and splitting to separate migrations should not be necessary.

@LKay LKay changed the title Can't create trigger on the table form the same migration Can't create trigger on the table from the same migration Jan 28, 2022
@olirice
Copy link
Owner

olirice commented Jan 28, 2022

Hi, this is the same challenge as #41

This seems like a bug and splitting to separate migrations should not be necessary.

I agree that it's clunky. The summary is that alembic_utils can not refer to alembic entities in a single migration due to the way it has to do dependency resolution. The difference is, alembic inspects the database and diffs the sqla models with the db schema in python while alembic_utils

  • snapshots the database state
  • creates the local entity (PGTrigger) in the database inside a transaction
  • snapshots the newly created entity
  • compares the before/after
  • rolls back the transaction

so in the case of a trigger being created in the same migration of as the table it refers to, things break down at the

  • creates the local entity (PGTrigger) in the database inside a transaction

step b/c the table doesn't exist

As a workaround the migration has to be broken up into two, first with all entities and second with only triggers attached to the tables

yes, this is currently the best workaround

@sambrilleman
Copy link

I have a similar issue to the above, but related to a PGView referencing a newly added column in an sqlalchemy table, and both trying to be handled in the same migration. The main part of the traceback in my case being:

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column "my_new_column" does not exist

As a workaround the migration has to be broken up into two, first with all entities and second with only triggers attached to the tables

yes, this is currently the best workaround

Re this suggestion ☝️ - can you recommend a way to do this in a single alembic revision --autogenerate ? That is, is there a way I can still have the registered entities, but then set up part of my env.py file so that each "auto-generated" revision runs in a two-step process... first ignoring the registered entities, then a second time not ignoring the registered entities? (or perhaps this doesn't make sense, if for e.g. alembic upgrade needs to be run in between...?)

@olirice
Copy link
Owner

olirice commented Feb 23, 2022

first ignoring the registered entities, then a second time not ignoring the registered entities? (or perhaps this doesn't make sense, if for e.g. alembic upgrade needs to be run in between...?)

alembic requires the database to be up-to-date (no unapplied migrations) in order to autogenerate a new migration. If it weren't for that restriction, producing two would be a great option

can you recommend a way to do this in a single alembic revision --autogenerate

unfortunately, it isn't currently possible. The workaround would be to

  • update the sqla table
  • autogen a migration
  • update the view
  • autogen a migration

@Andrei-Pozolotin
Copy link

Andrei-Pozolotin commented Mar 27, 2023

perhaps it would be easier to switch this project to pglast:
a native postgres ast parser (rather then rendering with live postgres server)
that would also remove multiple hacks used with current py.parse template engine

https://github.com/lelit/pglast

https://github.com/pganalyze/libpg_query

@oc-ben-ellis
Copy link

The difference is, alembic inspects the database and diffs the sqla models with the db schema in python while alembic_utils

@olirice Is there a reason why alembic_utils couldn't take the same approach? Is it just easier this way?

@olirice
Copy link
Owner

olirice commented Jun 1, 2023

Tables, indexes, and (some) constraints are highly structured. That makes it possible to look at local definitions in python, and check in the database to see if they exist. They also can be broken down into a clear dependency tree by following foreign keys. Thats how alembic works

Functions, triggers, and views allows arbitrary SQL in their definitions. Much less structured. Postgres stores them internally as text, but that text isn't exactly what you provided. It parses and reformats the text so your local text blob does not match the text blog stored in e.g. the database's definition of a function.

The way alembic_utils checks to see if a functions definition has changed is to look at a function's current definition in the database, apply the local definition (in a transaction), check if the definition in the db changed, and then roll back the transaction.

Functions can also have references to other functions or tables in arbitrary and potentially dynamic SQL. More than one function can change at a time, so we have to try to solve the resolution order that allows all of the local declarative definitions to be applied in a single step.

Creating functions, views and triggers is fast and always uses few resources. For that reason, we can very quickly try to solve the resolution order inside transactions and ultimately roll them back while you wait for the migration to generate.

In contrast, the commands that alembic issues, might be extremely slow. For example, adding a new column with a default on a large table, creating an index, etc.

If we simulate the alembic events in the transaction the same way the database could lock up

Technically users shouldn't be autogenerating migrations while pointing at their production database, but it's clear that a large portion do from reading issues in alembic/alembic_utils

I'd be open to (and would love to use) a contribution that made it configurable to execute the alembic ops prior to running alembic_utils' diffing as long as it defaulted to off, but unfortunately its a larger feature than I currently have time to tackle

@oc-ben-ellis
Copy link

@olirice Thanks for the explanation :)

@firdavsik
Copy link

firdavsik commented Apr 1, 2024

image
Call register_entities at the bottom of env.py (after run_migrations.....)

It is enough 😃

@guyarad
Copy link

guyarad commented Apr 16, 2024

@firdavsik

Call register_entities at the bottom of env.py (after run_migrations.....)

This doesn't work for me. It simply didn't consider the registered entities during the migration at all - so it succeeded, but it didn't add the alembic-utils-entities.

Can you share more about your process/configuration?

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

No branches or pull requests

7 participants