-
Notifications
You must be signed in to change notification settings - Fork 48
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
Materialized view change detection fails if upstream view has changed #116
Comments
In complex situations like this with multiple nested dependencies there are many things that can go wrong As a rule of thumb, only change 1 view/matview at a time between auto-gen runs. You'll end up with more migrations, but it'll succeed more of the time. If that doesn't work in this case, I'd recommend importing each
the order in |
The error happens even if there is only one view that is changed for the migration being generated. In the case I'm describing, only |
This also happens if its just a plain view that depends on a view that depends on another that changed. CLI output
If I remove |
Reproducible test case import pytest
from sqlalchemy.exc import ProgrammingError
from alembic_utils.exceptions import SQLParseFailure
from alembic_utils.pg_view import PGView
from alembic_utils.replaceable_entity import register_entities
from alembic_utils.testbase import TEST_VERSIONS_ROOT, run_alembic_command
def test_update_nested_view_revision(engine) -> None:
TEST_VIEW_1 = PGView(schema="public", signature="view1", definition="select 1 as one")
TEST_VIEW_2 = PGView(
schema="public", signature="view2", definition="select one from public.view1"
)
TEST_VIEW_3 = PGView(
schema="public", signature="view3", definition="select one from public.view2"
)
# Create the view outside of a revision
with engine.begin() as connection:
connection.execute(TEST_VIEW_1.to_sql_statement_create())
connection.execute(TEST_VIEW_2.to_sql_statement_create())
connection.execute(TEST_VIEW_3.to_sql_statement_create())
# Update definition of TO_UPPER
UPDATED_TEST_VIEW_1 = PGView(TEST_VIEW_1.schema, TEST_VIEW_1.signature, "select 2 as one")
register_entities([UPDATED_TEST_VIEW_1, TEST_VIEW_2, TEST_VIEW_3], entity_types=[PGView])
# Autogenerate a new migration
# It should detect the change we made and produce a "replace_function" statement
output = run_alembic_command(
engine=engine,
command="revision",
command_kwargs={"autogenerate": True, "rev_id": "2", "message": "replace"},
)
migration_replace_path = TEST_VERSIONS_ROOT / "2_replace.py"
with migration_replace_path.open() as migration_file:
import pdb
pdb.set_trace()
migration_contents = migration_file.read()
assert "op.replace_entity" in migration_contents
assert "op.create_entity" not in migration_contents
assert "op.drop_entity" not in migration_contents
assert "from alembic_utils.pg_view import PGView" in migration_contents
# Execute upgrade
run_alembic_command(engine=engine, command="upgrade", command_kwargs={"revision": "head"})
# Execute Downgrade
run_alembic_command(engine=engine, command="downgrade", command_kwargs={"revision": "base"}) Output
Trace
I had a look and transitive dependencies are not detected during dependency resolution. I'd suggest resolving that migration manually using the steps from my previous comment |
Would it be a difficult change to update the dependency resolution mechanism to identify transitive dependencies? I could attempt that change if you're open to PRs? |
I'm definitely open to PRs but it is a very complex problem to improve dependency resolution without making resolution time balloon out of control Here are good places to review alembic_utils/src/alembic_utils/depends.py Lines 13 to 55 in ed303f5
alembic_utils/src/alembic_utils/replaceable_entity.py Lines 291 to 353 in ed303f5
alembic_utils/src/alembic_utils/replaceable_entity.py Lines 98 to 122 in ed303f5
alembic_utils/src/alembic_utils/replaceable_entity.py Lines 156 to 178 in ed303f5
|
I am trying to add support for indexes on Materialized Views and I am actually running into a similar issue with simulation and definition comparison. Perhaps I'm reading the code wrong, but is the best way to compare the definitions by simulating the additions to the database? |
I'm having issues autogenerating a migration.
I have a Materialized view (
mat_view
) that is dependent on another view (view1
) which is dependent on a view (view2
) that changed.view2
's changes are found and a replace op is properly detected as needed.view1
is correctly identified as not needing changes. During change detection ofmat_view
however, it throws asqlalchemy.exc.ProgrammingError
sayingmat_view
doesn't exist, and during the handling of that error, the db api throws another error saying thatview1
(the one that depends on the one that changed) doesn't exist.I've made sure that the views are registered in dependent order.
I've been able to work around it by generating a migration with
mat_view
removed from the registered entities, which generates a replace op forview2
and a drop op format_view
. I then manually remove the drop op format_view
and it applies fine. That is only feasible at small scales though.The text was updated successfully, but these errors were encountered: