From d1a3fb953b12c3dc41a43ad5679e9f9c781f8417 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 17 Mar 2023 13:56:58 -0700 Subject: [PATCH 1/3] fix: prevent ForeignKeyViolation error on delete --- superset/charts/commands/delete.py | 4 ++++ superset/connectors/sqla/models.py | 2 +- superset/datasets/commands/delete.py | 4 ++++ superset/models/slice.py | 2 +- 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/superset/charts/commands/delete.py b/superset/charts/commands/delete.py index cb6644c711c4..1dae6a94b5ea 100644 --- a/superset/charts/commands/delete.py +++ b/superset/charts/commands/delete.py @@ -47,6 +47,10 @@ def run(self) -> Model: self.validate() try: Dashboard.clear_cache_for_slice(slice_id=self._model_id) + # Even thought SQLAlchemy should in theory delete rows from the association + # table, sporadically Superset will error because the rows are not deleted. + # Let's do it manually here to prevent the error. + self._model.owners = [] chart = ChartDAO.delete(self._model) except DAODeleteFailedError as ex: logger.exception(ex.exception) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index b76e423caf44..36aa5e4fe0e0 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -520,7 +520,7 @@ def data(self) -> Dict[str, Any]: metadata, Column("id", Integer, primary_key=True), Column("user_id", Integer, ForeignKey("ab_user.id")), - Column("table_id", Integer, ForeignKey("tables.id")), + Column("table_id", Integer, ForeignKey("tables.id", ondelete="CASCADE")), ) diff --git a/superset/datasets/commands/delete.py b/superset/datasets/commands/delete.py index 6f9156795813..d457ad4e34cc 100644 --- a/superset/datasets/commands/delete.py +++ b/superset/datasets/commands/delete.py @@ -44,6 +44,10 @@ def __init__(self, model_id: int): def run(self) -> Model: self.validate() try: + # Even thought SQLAlchemy should in theory delete rows from the association + # table, sporadically Superset will error because the rows are not deleted. + # Let's do it manually here to prevent the error. + self._model.owners = [] dataset = DatasetDAO.delete(self._model, commit=False) db.session.commit() except (SQLAlchemyError, DAODeleteFailedError) as ex: diff --git a/superset/models/slice.py b/superset/models/slice.py index 9ab4039a93d5..12e9ac404e4b 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -59,7 +59,7 @@ metadata, Column("id", Integer, primary_key=True), Column("user_id", Integer, ForeignKey("ab_user.id")), - Column("slice_id", Integer, ForeignKey("slices.id")), + Column("slice_id", Integer, ForeignKey("slices.id", ondelete="CASCADE")), ) logger = logging.getLogger(__name__) From 2f756c7ecd9e8d6a35d6f643c7ad26a7747173c0 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 17 Mar 2023 14:06:03 -0700 Subject: [PATCH 2/3] Fix type --- superset/charts/commands/delete.py | 5 +++-- superset/datasets/commands/delete.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/superset/charts/commands/delete.py b/superset/charts/commands/delete.py index 1dae6a94b5ea..4c636f0433a7 100644 --- a/superset/charts/commands/delete.py +++ b/superset/charts/commands/delete.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. import logging -from typing import Optional +from typing import cast, Optional from flask_appbuilder.models.sqla import Model from flask_babel import lazy_gettext as _ @@ -45,9 +45,10 @@ def __init__(self, model_id: int): def run(self) -> Model: self.validate() + self._model = cast(Slice, self._model) try: Dashboard.clear_cache_for_slice(slice_id=self._model_id) - # Even thought SQLAlchemy should in theory delete rows from the association + # Even though SQLAlchemy should in theory delete rows from the association # table, sporadically Superset will error because the rows are not deleted. # Let's do it manually here to prevent the error. self._model.owners = [] diff --git a/superset/datasets/commands/delete.py b/superset/datasets/commands/delete.py index d457ad4e34cc..1487f1028b3b 100644 --- a/superset/datasets/commands/delete.py +++ b/superset/datasets/commands/delete.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. import logging -from typing import Optional +from typing import cast, Optional from flask_appbuilder.models.sqla import Model from sqlalchemy.exc import SQLAlchemyError @@ -43,8 +43,9 @@ def __init__(self, model_id: int): def run(self) -> Model: self.validate() + self._model = cast(SqlaTable, self._model) try: - # Even thought SQLAlchemy should in theory delete rows from the association + # Even though SQLAlchemy should in theory delete rows from the association # table, sporadically Superset will error because the rows are not deleted. # Let's do it manually here to prevent the error. self._model.owners = [] From 2d9abb06614bacf48cd8815094ac0717dccf30f7 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 21 Mar 2023 11:46:51 -0700 Subject: [PATCH 3/3] Remove ondelete --- superset/connectors/sqla/models.py | 2 +- superset/models/slice.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 36aa5e4fe0e0..b76e423caf44 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -520,7 +520,7 @@ def data(self) -> Dict[str, Any]: metadata, Column("id", Integer, primary_key=True), Column("user_id", Integer, ForeignKey("ab_user.id")), - Column("table_id", Integer, ForeignKey("tables.id", ondelete="CASCADE")), + Column("table_id", Integer, ForeignKey("tables.id")), ) diff --git a/superset/models/slice.py b/superset/models/slice.py index 12e9ac404e4b..9ab4039a93d5 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -59,7 +59,7 @@ metadata, Column("id", Integer, primary_key=True), Column("user_id", Integer, ForeignKey("ab_user.id")), - Column("slice_id", Integer, ForeignKey("slices.id", ondelete="CASCADE")), + Column("slice_id", Integer, ForeignKey("slices.id")), ) logger = logging.getLogger(__name__)