michael-s-molina commented on code in PR #32759:
URL: https://github.com/apache/superset/pull/32759#discussion_r2004201972


##########
superset/migrations/shared/utils.py:
##########
@@ -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 constraint_names is not None:
+            constraint_names = list(set(constraint_names) & existing_fks)
+        else:
+            constraint_names = list(existing_fks)
+
+        for fk_name in constraint_names:

Review Comment:
   ```suggestion
           if fk_names is not None:
               fk_names = list(set(fk_names) & existing_fks)
           else:
               fk_names = list(existing_fks)
   
           for fk_name in fk_names:
   ```



##########
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, constraint_names: Optional[list[str]] = None

Review Comment:
   ```suggestion
       table_name: str, fk_names: list[str] | None = None
   ```



##########
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, constraint_names: Optional[list[str]] = 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 constraint_names: Optional list of specific foreign key names to 
drop.

Review Comment:
   ```suggestion
       :param fk_names: Optional list of specific foreign key names to drop.
   ```



##########
superset/migrations/shared/utils.py:
##########
@@ -406,3 +417,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(
+    constraint_name: str,

Review Comment:
   Could you also rename to `fk_name` here? Constraint is more generic and can 
be a foreign key, unique key, not null, etc.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to