From 1c6bf334ac1234876dec560aa4ec18b88ca8bb7a Mon Sep 17 00:00:00 2001 From: bogdan kyryliuk Date: Thu, 24 Sep 2020 17:37:53 -0700 Subject: [PATCH 1/3] Fix table existance validation function --- superset/connectors/sqla/models.py | 2 +- superset/datasets/commands/create.py | 4 ++-- superset/datasets/dao.py | 10 ++++++--- tests/datasets/api_tests.py | 33 +++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index df07ccad67a89..7f8fcfa1d2808 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -453,7 +453,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at owner_class = security_manager.user_model __tablename__ = "tables" - __table_args__ = (UniqueConstraint("database_id", "table_name"),) + __table_args__ = (UniqueConstraint("database_id", "schema", "table_name"),) table_name = Column(String(250), nullable=False) main_dttm_col = Column(String(250)) diff --git a/superset/datasets/commands/create.py b/superset/datasets/commands/create.py index 8dd0f4a2b8a88..80e86db5a2213 100644 --- a/superset/datasets/commands/create.py +++ b/superset/datasets/commands/create.py @@ -70,11 +70,11 @@ def validate(self) -> None: exceptions: List[ValidationError] = list() database_id = self._properties["database"] table_name = self._properties["table_name"] - schema = self._properties.get("schema", "") + schema = self._properties.get("schema", None) owner_ids: Optional[List[int]] = self._properties.get("owners") # Validate uniqueness - if not DatasetDAO.validate_uniqueness(database_id, table_name): + if not DatasetDAO.validate_uniqueness(database_id, schema, table_name): exceptions.append(DatasetExistsValidationError(table_name)) # Validate/Populate database diff --git a/superset/datasets/dao.py b/superset/datasets/dao.py index 2f416db0ce1d3..b34b3ef7ea8da 100644 --- a/superset/datasets/dao.py +++ b/superset/datasets/dao.py @@ -74,7 +74,9 @@ def get_related_objects(database_id: int) -> Dict[str, Any]: return dict(charts=charts, dashboards=dashboards) @staticmethod - def validate_table_exists(database: Database, table_name: str, schema: str) -> bool: + def validate_table_exists( + database: Database, table_name: str, schema: Optional[str] + ) -> bool: try: database.get_table(table_name, schema=schema) return True @@ -83,9 +85,11 @@ def validate_table_exists(database: Database, table_name: str, schema: str) -> b return False @staticmethod - def validate_uniqueness(database_id: int, name: str) -> bool: + def validate_uniqueness(database_id: int, schema: Optional[str], name: str) -> bool: dataset_query = db.session.query(SqlaTable).filter( - SqlaTable.table_name == name, SqlaTable.database_id == database_id + SqlaTable.table_name == name, + SqlaTable.schema == schema, + SqlaTable.database_id == database_id, ) return not db.session.query(dataset_query.exists()).scalar() diff --git a/tests/datasets/api_tests.py b/tests/datasets/api_tests.py index 56f9ef84675c6..6bd7e86c6f369 100644 --- a/tests/datasets/api_tests.py +++ b/tests/datasets/api_tests.py @@ -32,10 +32,11 @@ ) from superset.extensions import db, security_manager from superset.models.core import Database -from superset.utils.core import get_example_database, get_main_database +from superset.utils.core import backend, get_example_database, get_main_database from superset.utils.dict_import_export import export_to_dict from superset.views.base import generate_download_headers from tests.base_tests import SupersetTestCase +from tests.conftest import CTAS_SCHEMA_NAME class TestDatasetApi(SupersetTestCase): @@ -387,6 +388,36 @@ def test_create_dataset_validate_uniqueness(self): data, {"message": {"table_name": ["Datasource birth_names already exists"]}} ) + def test_create_dataset_same_name_different_schema(self): + if backend() == "sqlite": + # sqlite doesn't support schemas + return + + example_db = get_example_database() + example_db.get_sqla_engine().execute( + f"CREATE TABLE {CTAS_SCHEMA_NAME}.birth_names AS SELECT 2 as two" + ) + + self.login(username="admin") + table_data = { + "database": example_db.id, + "schema": CTAS_SCHEMA_NAME, + "table_name": "birth_names", + } + + uri = "api/v1/dataset/" + rv = self.post_assert_metric(uri, table_data, "post") + self.assertEqual(rv.status_code, 201) + + # cleanup + data = json.loads(rv.data.decode("utf-8")) + uri = f'api/v1/dataset/{data.get("id")}' + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 200) + example_db.get_sqla_engine().execute( + f"DROP TABLE {CTAS_SCHEMA_NAME}.birth_names" + ) + def test_create_dataset_validate_database(self): """ Dataset API: Test create dataset validate database exists From 76f1e1cf0e4b8df1ec4142d768467f8968b6e2e8 Mon Sep 17 00:00:00 2001 From: bogdan kyryliuk Date: Fri, 25 Sep 2020 13:03:27 -0700 Subject: [PATCH 2/3] Drop left over table name index in mysql db --- ...98_fix_table_unique_constraint_in_mysql.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 superset/migrations/versions/18532d70ab98_fix_table_unique_constraint_in_mysql.py diff --git a/superset/migrations/versions/18532d70ab98_fix_table_unique_constraint_in_mysql.py b/superset/migrations/versions/18532d70ab98_fix_table_unique_constraint_in_mysql.py new file mode 100644 index 0000000000000..38224017cead7 --- /dev/null +++ b/superset/migrations/versions/18532d70ab98_fix_table_unique_constraint_in_mysql.py @@ -0,0 +1,42 @@ +# 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. +"""Delete table_name unique constraint in mysql + +Revision ID: 18532d70ab98 +Revises: e5ef6828ac4e +Create Date: 2020-09-25 10:56:13.711182 + +""" + +# revision identifiers, used by Alembic. +revision = "18532d70ab98" +down_revision = "e5ef6828ac4e" + +from alembic import op + + +def upgrade(): + try: + # index only exists in mysql db + with op.get_context().autocommit_block(): + op.drop_constraint("table_name", "tables", type_="unique") + except Exception as ex: + print(ex) + + +def downgrade(): + pass From d423f8eafb9c96bcc85b4190bd1c491e84c95d7f Mon Sep 17 00:00:00 2001 From: bogdan kyryliuk Date: Mon, 28 Sep 2020 17:28:17 -0700 Subject: [PATCH 3/3] Do not modify model --- superset/connectors/sqla/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 7f8fcfa1d2808..df07ccad67a89 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -453,7 +453,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at owner_class = security_manager.user_model __tablename__ = "tables" - __table_args__ = (UniqueConstraint("database_id", "schema", "table_name"),) + __table_args__ = (UniqueConstraint("database_id", "table_name"),) table_name = Column(String(250), nullable=False) main_dttm_col = Column(String(250))