This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 5.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 770b3e17bb917c713d594e7b24119e2079f6cf10 Author: Antonio Rivero <[email protected]> AuthorDate: Wed Mar 19 22:47:26 2025 +0100 fix(migrations): fix foreign keys to match FAB 4.6.0 tables (#32759) (cherry picked from commit 376a1f49d339c393af678b12ad74ae91a72c94fc) --- superset/migrations/shared/utils.py | 81 +++++++++++++-- ...46_32bf93dfe2a4_add_on_cascade_in_fab_tables.py | 111 +++++++++++++++++++++ 2 files changed, 184 insertions(+), 8 deletions(-) diff --git a/superset/migrations/shared/utils.py b/superset/migrations/shared/utils.py index b0a460f588..18166b047d 100644 --- a/superset/migrations/shared/utils.py +++ b/superset/migrations/shared/utils.py @@ -193,12 +193,16 @@ def has_table(table_name: str) -> bool: return table_exists -def drop_fks_for_table(table_name: str) -> None: +def drop_fks_for_table( + table_name: str, foreign_key_names: list[str] | None = None +) -> None: """ - Drop all foreign key constraints for a table if it exist and the database - is not sqlite. + Drop specific or all foreign key constraints for a table + if they exist and the database is not sqlite. - :param table_name: The table name to drop foreign key constraints for + :param table_name: The table name to drop foreign key constraints from + :param foreign_key_names: Optional list of specific foreign key names to drop. + If None is provided, all will be dropped. """ connection = op.get_bind() inspector = Inspector.from_engine(connection) @@ -207,12 +211,19 @@ def drop_fks_for_table(table_name: str) -> None: return # sqlite doesn't like constraints if has_table(table_name): - foreign_keys = inspector.get_foreign_keys(table_name) - for fk in foreign_keys: + existing_fks = {fk["name"] for fk in inspector.get_foreign_keys(table_name)} + + # What to delete based on whether the list was passed + if foreign_key_names is not None: + foreign_key_names = list(set(foreign_key_names) & existing_fks) + else: + foreign_key_names = list(existing_fks) + + for fk_name in foreign_key_names: logger.info( - f"Dropping foreign key {GREEN}{fk['name']}{RESET} from table {GREEN}{table_name}{RESET}..." # noqa: E501 + f"Dropping foreign key {GREEN}{fk_name}{RESET} from table {GREEN}{table_name}{RESET}..." # noqa: E501 ) - op.drop_constraint(fk["name"], table_name, type_="foreignkey") + op.drop_constraint(fk_name, table_name, type_="foreignkey") def create_table(table_name: str, *columns: SchemaItem) -> None: @@ -398,3 +409,57 @@ def drop_index(table_name: str, index_name: str) -> None: ) op.drop_index(table_name=table_name, index_name=index_name) + + +def create_fks_for_table( + foreign_key_name: str, + table_name: str, + referenced_table: str, + local_cols: list[str], + remote_cols: list[str], + ondelete: Optional[str] = None, +) -> None: + """ + Create a foreign key constraint for a table, ensuring compatibility with sqlite. + + :param foreign_key_name: Foreign key constraint name. + :param table_name: The name of the table where the foreign key will be created. + :param referenced_table: The table the FK references. + :param local_cols: Column names in the current table. + :param remote_cols: Column names in the referenced table. + :param ondelete: (Optional) The ON DELETE action (e.g., "CASCADE", "SET NULL"). + """ + connection = op.get_bind() + + if not has_table(table_name): + logger.warning( + f"Table {LRED}{table_name}{RESET} does not exist. Skipping foreign key creation." # noqa: E501 + ) + return + + if isinstance(connection.dialect, SQLiteDialect): + # SQLite requires batch mode since ALTER TABLE is limited + with op.batch_alter_table(table_name) as batch_op: + logger.info( + f"Creating foreign key {GREEN}{foreign_key_name}{RESET} on table {GREEN}{table_name}{RESET} (SQLite mode)..." # noqa: E501 + ) + batch_op.create_foreign_key( + foreign_key_name, + referenced_table, + local_cols, + remote_cols, + ondelete=ondelete, + ) + else: + # Standard FK creation for other databases + logger.info( + f"Creating foreign key {GREEN}{foreign_key_name}{RESET} on table {GREEN}{table_name}{RESET}..." # noqa: E501 + ) + op.create_foreign_key( + foreign_key_name, + table_name, + referenced_table, + local_cols, + remote_cols, + ondelete=ondelete, + ) diff --git a/superset/migrations/versions/2025-03-19_17-46_32bf93dfe2a4_add_on_cascade_in_fab_tables.py b/superset/migrations/versions/2025-03-19_17-46_32bf93dfe2a4_add_on_cascade_in_fab_tables.py new file mode 100644 index 0000000000..e7a9a3d75c --- /dev/null +++ b/superset/migrations/versions/2025-03-19_17-46_32bf93dfe2a4_add_on_cascade_in_fab_tables.py @@ -0,0 +1,111 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Add on cascade to foreign keys in ab_permission_view_role and ab_user_role + +Revision ID: 32bf93dfe2a4 +Revises: 94e7a3499973 +Create Date: 2025-03-19 17:46:25.702610 + +""" + +from superset.migrations.shared.utils import create_fks_for_table, drop_fks_for_table + +# revision identifiers, used by Alembic. +revision = "32bf93dfe2a4" +down_revision = "94e7a3499973" + + +def upgrade(): + drop_fks_for_table( + "ab_permission_view_role", + [ + "ab_permission_view_role_permission_view_id_fkey", + "ab_permission_view_role_role_id_fkey", + ], + ) + create_fks_for_table( + "ab_permission_view_role_role_id_fkey", + "ab_permission_view_role", + "ab_role", + ["role_id"], + ["id"], + ondelete="CASCADE", + ) + create_fks_for_table( + "ab_permission_view_role_permission_view_id_fkey", + "ab_permission_view_role", + "ab_permission_view", + ["permission_view_id"], + ["id"], + ondelete="CASCADE", + ) + + drop_fks_for_table( + "ab_user_role", + ["ab_user_role_user_id_fkey", "ab_user_role_role_id_fkey"], + ) + create_fks_for_table( + "ab_user_role_user_id_fkey", + "ab_user_role", + "ab_user", + ["user_id"], + ["id"], + ondelete="CASCADE", + ) + create_fks_for_table( + "ab_user_role_role_id_fkey", + "ab_user_role", + "ab_role", + ["role_id"], + ["id"], + ondelete="CASCADE", + ) + + +def downgrade(): + drop_fks_for_table( + "ab_permission_view_role", + [ + "ab_permission_view_role_permission_view_id_fkey", + "ab_permission_view_role_role_id_fkey", + ], + ) + create_fks_for_table( + "ab_permission_view_role_permission_view_id_fkey", + "ab_permission_view_role", + "ab_permission_view", + ["permission_view_id"], + ["id"], + ) + create_fks_for_table( + "ab_permission_view_role_role_id_fkey", + "ab_permission_view_role", + "ab_role", + ["role_id"], + ["id"], + ) + + drop_fks_for_table( + "ab_user_role", + ["ab_user_role_user_id_fkey", "ab_user_role_role_id_fkey"], + ) + create_fks_for_table( + "ab_user_role_user_id_fkey", "ab_user_role", "ab_user", ["user_id"], ["id"] + ) + create_fks_for_table( + "ab_user_role_role_id_fkey", "ab_user_role", "ab_role", ["role_id"], ["id"] + )
