This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a114b4cc468 fix(import): skip redundant has_table check when dataset
import has no data URI (#43808)
a114b4cc468 is described below
commit a114b4cc468347cd4dfae962b4e4d635f11165b6
Author: Kunal <[email protected]>
AuthorDate: Wed Sep 9 01:57:40 2026 +0530
fix(import): skip redundant has_table check when dataset import has no data
URI (#43808)
Co-authored-by: Kunal8954 <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
---
superset/commands/dataset/importers/v1/utils.py | 34 +++--
.../datasets/commands/importers/v1/import_test.py | 142 +++++++++++++++++++++
2 files changed, 164 insertions(+), 12 deletions(-)
diff --git a/superset/commands/dataset/importers/v1/utils.py
b/superset/commands/dataset/importers/v1/utils.py
index 844ac02b8f2..39dd81b476c 100644
--- a/superset/commands/dataset/importers/v1/utils.py
+++ b/superset/commands/dataset/importers/v1/utils.py
@@ -555,19 +555,29 @@ def import_dataset( # noqa: C901
except SupersetSecurityException as ex:
raise DatasetAccessDeniedError() from ex
- try:
- table_exists = dataset.database.has_table(
- Table(dataset.table_name, dataset.schema, dataset.catalog),
- )
- except Exception: # pylint: disable=broad-except
- # MySQL doesn't play nice with GSheets table names
- logger.warning(
- "Couldn't check if table %s exists, assuming it does",
dataset.table_name
- )
- table_exists = True
+ # `has_table` opens a live connection to the target database to run a
+ # schema-introspection query. Its result is only ever consulted below to
+ # decide whether to call `load_data`, which itself is a no-op unless
+ # `data_uri` is set - so for imports that don't carry inline data (the
+ # common case when bulk-importing dataset *metadata*, e.g. hundreds of
+ # datasets at once), this was an unconditional, unnecessary round trip
+ # to every target database on every single dataset, and a major
+ # contributor to bulk imports timing out.
+ if data_uri:
+ try:
+ table_exists = dataset.database.has_table(
+ Table(dataset.table_name, dataset.schema, dataset.catalog),
+ )
+ except Exception: # pylint: disable=broad-except
+ # MySQL doesn't play nice with GSheets table names
+ logger.warning(
+ "Couldn't check if table %s exists, assuming it does",
+ dataset.table_name,
+ )
+ table_exists = True
- if data_uri and (not table_exists or force_data):
- load_data(data_uri, dataset, dataset.database)
+ if not table_exists or force_data:
+ load_data(data_uri, dataset, dataset.database)
if user:
from superset.subjects.utils import get_user_subject
diff --git a/tests/unit_tests/datasets/commands/importers/v1/import_test.py
b/tests/unit_tests/datasets/commands/importers/v1/import_test.py
index c3e65bb1055..bebc5687527 100644
--- a/tests/unit_tests/datasets/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py
@@ -773,6 +773,148 @@ def test_import_dataset_no_folder(mocker: MockerFixture,
session: Session) -> No
assert sqla_table.folders is None
+def test_import_dataset_skips_has_table_check_without_data_uri(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Importing a dataset with no ``data`` URI should never call
+ ``Database.has_table`` - its result only ever gates ``load_data``, which
+ is already a no-op when there's no data URI to load. Skipping the call
+ avoids an unnecessary round trip to the target database on every
+ imported dataset, which is what made bulk imports of many datasets slow
+ enough to hit the gunicorn worker timeout.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ has_table = mocker.patch.object(Database, "has_table")
+
+ engine = db.session.get_bind()
+ SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
+
+ database = Database(database_name="my_database",
sqlalchemy_uri="sqlite://")
+ db.session.add(database)
+ db.session.flush()
+
+ config = {
+ "table_name": "no_data_table",
+ "main_dttm_col": None,
+ "description": None,
+ "default_endpoint": None,
+ "offset": 0,
+ "cache_timeout": None,
+ "schema": None,
+ "sql": None,
+ "params": None,
+ "template_params": None,
+ "filter_select_enabled": False,
+ "fetch_values_predicate": None,
+ "extra": None,
+ "uuid": uuid.uuid4(),
+ "metrics": [],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+
+ import_dataset(config)
+
+ has_table.assert_not_called()
+
+
+def test_import_dataset_checks_has_table_with_data_uri(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ When a ``data`` URI is present, ``Database.has_table`` should still be
+ consulted to decide whether the data needs to be (re-)loaded. A table that
+ already exists means the data is there, so it must not be re-loaded.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ has_table = mocker.patch.object(Database, "has_table", return_value=True)
+ load_data =
mocker.patch("superset.commands.dataset.importers.v1.utils.load_data")
+
+ engine = db.session.get_bind()
+ SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
+
+ database = Database(database_name="my_database",
sqlalchemy_uri="sqlite://")
+ db.session.add(database)
+ db.session.flush()
+
+ config = {
+ "table_name": "has_data_table",
+ "main_dttm_col": None,
+ "description": None,
+ "default_endpoint": None,
+ "offset": 0,
+ "cache_timeout": None,
+ "schema": None,
+ "sql": None,
+ "params": None,
+ "template_params": None,
+ "filter_select_enabled": False,
+ "fetch_values_predicate": None,
+ "extra": None,
+ "uuid": uuid.uuid4(),
+ "metrics": [],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ "data": "https://example.com/data.csv";,
+ }
+
+ import_dataset(config)
+
+ has_table.assert_called_once()
+ load_data.assert_not_called()
+
+
+def test_import_dataset_loads_data_when_table_is_missing(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ When a ``data`` URI is present and the target table doesn't exist yet, the
+ data must actually be loaded. This is the case ``has_table`` exists to
+ detect, and the one an inverted or misindented condition would silently
+ skip while still satisfying the checks above.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ has_table = mocker.patch.object(Database, "has_table", return_value=False)
+ load_data =
mocker.patch("superset.commands.dataset.importers.v1.utils.load_data")
+
+ engine = db.session.get_bind()
+ SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
+
+ database = Database(database_name="my_database",
sqlalchemy_uri="sqlite://")
+ db.session.add(database)
+ db.session.flush()
+
+ config = {
+ "table_name": "missing_data_table",
+ "main_dttm_col": None,
+ "description": None,
+ "default_endpoint": None,
+ "offset": 0,
+ "cache_timeout": None,
+ "schema": None,
+ "sql": None,
+ "params": None,
+ "template_params": None,
+ "filter_select_enabled": False,
+ "fetch_values_predicate": None,
+ "extra": None,
+ "uuid": uuid.uuid4(),
+ "metrics": [],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ "data": "https://example.com/data.csv";,
+ }
+
+ import_dataset(config)
+
+ has_table.assert_called_once()
+ load_data.assert_called_once()
+
+
def
test_import_dataset_rejects_non_default_catalog_when_multi_catalog_disabled(
mocker: MockerFixture, session: Session
) -> None:
