You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/07 18:15:31 UTC
(superset) branch master updated: fix(datasets): preserve metric/column uuids on dataset export (#42393)
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 c901f016930 fix(datasets): preserve metric/column uuids on dataset export (#42393)
c901f016930 is described below
commit c901f0169309a9f8cab98849e83fab37287a390b
Author: Joe Li <jo...@preset.io>
AuthorDate: Fri Aug 7 11:15:24 2026 -0700
fix(datasets): preserve metric/column uuids on dataset export (#42393)
Co-authored-by: Claude Opus 4.8 <no...@anthropic.com>
---
UPDATING.md | 2 +
superset/commands/dataset/importers/v1/utils.py | 17 +
superset/datasets/schemas.py | 32 ++
superset/models/helpers.py | 95 ++++-
.../integration_tests/databases/commands_tests.py | 24 ++
tests/integration_tests/datasets/commands_tests.py | 13 +
tests/unit_tests/datasets/commands/export_test.py | 84 +++-
.../datasets/commands/importers/v1/import_test.py | 447 +++++++++++++++++++++
8 files changed, 707 insertions(+), 7 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 445061aaf0e..873af4419ee 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -24,6 +24,8 @@ assists people when migrating to a new version.
## Next
+- [42393](https://github.com/apache/superset/pull/42393): Exported dataset YAML now carries a `uuid` for each metric and column so that custom folder assignments (which reference metrics/columns by UUID) survive an import into another workspace. This affects any export bundle that contains datasets, not just a dataset export: chart, dashboard, database and full-asset exports all embed the same dataset YAML, so a dashboard exported from this release also fails to import into an older one [...]
+
### Soft delete is on by default, and purging is live
`SOFT_DELETE` now ships **on** (`DEFAULT_FEATURE_FLAGS`), so deleting a
diff --git a/superset/commands/dataset/importers/v1/utils.py b/superset/commands/dataset/importers/v1/utils.py
index cea8611a4db..266f1495677 100644
--- a/superset/commands/dataset/importers/v1/utils.py
+++ b/superset/commands/dataset/importers/v1/utils.py
@@ -43,6 +43,7 @@ from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES
from superset.daos.dataset import DatasetDAO
from superset.exceptions import SupersetSecurityException
from superset.models.core import Database
+from superset.models.helpers import ChildMultipleResultsFound
from superset.sql.parse import Table
from superset.utils import json
from superset.utils.core import get_user
@@ -442,6 +443,22 @@ def import_dataset( # noqa: C901
# import recursively to include columns and metrics
try:
dataset = SqlaTable.import_from_dict(config, recursive=True, sync=sync)
+ except ChildMultipleResultsFound as ex:
+ # An ambiguous *child* (metric/column) lookup. Unlike the dataset-level
+ # case handled below, this is raised after the dataset's own fields were
+ # updated and after earlier siblings were already imported, so there is
+ # no unmodified row to fall back to — silently returning one would
+ # report success over a half-applied import (parent scalars written,
+ # children not synced, the ``sync`` deletion never run). Raise instead
+ # so the command's transaction wrapper rolls the whole thing back.
+ raise ImportFailedError(
+ f"Dataset {config['table_name']!r} (uuid {config['uuid']}) "
+ "could not be imported because one of its metrics or columns "
+ "matches two different existing ones — one by name and another by "
+ "UUID. The import was aborted so it is not applied partially. "
+ "Rename or delete the conflicting metric/column in the target "
+ "instance, or remove the UUID from the uploaded file, and retry."
+ ) from ex
except MultipleResultsFound as ex:
# Finding multiple results when importing a dataset only happens because initially # noqa: E501
# datasets were imported without schemas (eg, `examples.NULL.users`), and later
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index faa1e39592a..59aa2185c78 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -16,6 +16,7 @@
# under the License.
from datetime import datetime
from typing import Any
+from uuid import UUID
from dateutil.parser import isoparse
from flask_babel import lazy_gettext as _
@@ -284,6 +285,7 @@ class ImportV1ColumnSchema(Schema):
description = fields.String(allow_none=True)
python_date_format = fields.String(allow_none=True)
datetime_format = fields.String(allow_none=True)
+ uuid = fields.UUID(allow_none=True)
class ImportMetricCurrencySchema(Schema):
@@ -326,6 +328,7 @@ class ImportV1MetricSchema(Schema):
currency = CurrencyField(ImportMetricCurrencySchema, allow_none=True)
extra = fields.Dict(allow_none=True)
warning_text = fields.String(allow_none=True)
+ uuid = fields.UUID(allow_none=True)
class ImportV1DatasetSchema(Schema):
@@ -348,6 +351,35 @@ class ImportV1DatasetSchema(Schema):
return data
+ @validates_schema
+ def validate_unique_child_uuids(self, data: dict[str, Any], **kwargs: Any) -> None:
+ """
+ Reject a payload where two metrics (or two columns) share a UUID.
+
+ UUIDs are globally unique in the database, so such a payload cannot be
+ imported faithfully: the importer matches children within their parent
+ by name *or* UUID, so the second entry would match the first one and
+ overwrite it in place, silently collapsing two metrics/columns into one.
+ Only a hand-edited bundle can produce this — an export never does.
+ """
+ for key, singular in (("metrics", "metric"), ("columns", "column")):
+ seen: set[UUID] = set()
+ duplicates: set[UUID] = set()
+ for child in data.get(key) or []:
+ child_uuid = child.get("uuid")
+ if child_uuid is None:
+ continue
+ if child_uuid in seen:
+ duplicates.add(child_uuid)
+ seen.add(child_uuid)
+ if duplicates:
+ raise ValidationError(
+ f"Duplicate UUIDs found in {key}: "
+ f"{', '.join(sorted(str(dup) for dup in duplicates))}. "
+ f"Each {singular} must have a unique `uuid`.",
+ field_name=key,
+ )
+
table_name = fields.String(required=True)
main_dttm_col = fields.String(allow_none=True)
currency_code_column = fields.String(allow_none=True)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 18810e5e4de..bfeaca5a9ef 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -369,6 +369,27 @@ class UUIDMixin: # pylint: disable=too-few-public-methods
return str(self.uuid)[:8]
+class ChildMultipleResultsFound(MultipleResultsFound):
+ """
+ An ambiguous lookup raised while importing a *child* object.
+
+ ``ImportExportMixin.import_from_dict`` matches an incoming payload against
+ existing rows with a disjunction (unique-constraint match OR uuid match), so
+ a single payload entry can match two different DB rows — one by name and one
+ by uuid. When that happens for a top-level object it is raised before any
+ mutation, and callers may legitimately recover by skipping or returning the
+ existing row (see ``superset/commands/importers/v1/examples.py``).
+
+ When it happens for a *child* (``parent is not None``) the parent's fields
+ have already been updated and earlier siblings have already been imported,
+ so recovering by "returning the existing row" would report success over a
+ half-applied import. This subclass lets callers tell the two cases apart and
+ abort the transaction for the child case. It intentionally subclasses
+ ``MultipleResultsFound`` so existing ``except MultipleResultsFound`` handlers
+ keep working.
+ """
+
+
class ImportExportMixin(UUIDMixin):
export_parent: Optional[str] = None
# The name of the attribute
@@ -506,7 +527,32 @@ class ImportExportMixin(UUIDMixin):
try:
obj_query = db.session.query(cls).filter(and_(*filters))
obj = obj_query.one_or_none()
- except MultipleResultsFound:
+ except MultipleResultsFound as ex:
+ # Now that children carry a ``uuid``, this match is a disjunction
+ # (name-match OR uuid-match), so a payload child can match one DB row
+ # by name and a different one by uuid — e.g. metrics renamed/swapped
+ # in the UI, then an older export re-imported with overwrite.
+ #
+ # That stays a hard error on purpose. Resolving it by preferring the
+ # uuid match would rename the uuid-matched row while the name-matched
+ # row still holds that name, trading this clear failure for an opaque
+ # ``UNIQUE constraint failed: (table_id, <name>)`` at the next flush.
+ #
+ # The two cases are not equally recoverable, so they are raised as
+ # distinct types:
+ #
+ # * ``parent is None`` (top-level object) — nothing has been mutated
+ # yet, so a caller may legitimately skip or fall back to the
+ # existing row. See the legacy NULL-schema handling in
+ # ``superset/commands/dataset/importers/v1/utils.py`` (including
+ # its ``deleted_at`` rollback) and the ``continue`` in
+ # ``superset/commands/importers/v1/examples.py`` (issue #16051).
+ # * ``parent is not None`` (child object) — the parent's scalar
+ # fields were already updated and earlier siblings were already
+ # imported, so there is no "unmodified row" to fall back to.
+ # Raising ``ChildMultipleResultsFound`` lets the dataset importer
+ # fail the import atomically (the command's transaction rolls
+ # back) instead of reporting success over a half-applied state.
logger.error(
"Error importing %s \n %s \n %s",
cls.__name__,
@@ -514,8 +560,54 @@ class ImportExportMixin(UUIDMixin):
yaml.safe_dump(dict_rep),
exc_info=True,
)
+ if parent is not None:
+ raise ChildMultipleResultsFound(
+ f"{cls.__name__} matches more than one existing row under "
+ f"its parent {type(parent).__name__}"
+ ) from ex
raise
+ # A child ``uuid`` is globally unique, but the match above is scoped to
+ # this ``parent`` (and also matches on name). Importing a config as a
+ # clone — e.g. a dataset re-imported under an edited uuid while the
+ # original still exists — can leave the incoming child uuid owned by a
+ # different row. Writing it, whether on INSERT (new obj) or on an
+ # overwrite UPDATE (obj matched by name), would violate the ``uuid``
+ # unique constraint at flush. Drop the incoming uuid whenever it belongs
+ # to some other row so ``UUIDMixin`` keeps/assigns a distinct one,
+ # reverting to the pre-uuid-export behavior for that clone.
+ if parent is not None and "uuid" in dict_rep:
+ if dict_rep["uuid"] is None:
+ # The child import schemas accept ``uuid: null``. On the
+ # overwrite UPDATE that would write a literal NULL over an
+ # existing child's uuid — silently, since the column is nullable
+ # and ``unique`` permits repeated NULLs — leaving a child no
+ # folder can reference and no export can round-trip.
+ del dict_rep["uuid"]
+ else:
+ # Known limitation: this lookup goes through the normal session
+ # visibility filter, so for child classes that are soft-deletable
+ # it cannot see a soft-deleted owner of the incoming uuid. In
+ # practice the children that matter here (``TableColumn`` /
+ # ``SqlMetric``) are not soft-deletable; the only soft-deletable
+ # child registered anywhere is ``SqlaTable`` via ``Database``'s
+ # ``export_children = ["tables"]`` (the v0 legacy path), which is
+ # reachable only from a hand-crafted v0 file. There a colliding
+ # uuid owned by a soft-deleted dataset would be kept and fail on
+ # the unique constraint at flush rather than being dropped here.
+ #
+ # Skip the lookup on the common idempotent re-import, where the
+ # name-matched ``obj`` already owns the incoming uuid: the guard
+ # would keep it anyway, so the query is pure waste on that path.
+ if obj is None or str(obj.uuid) != str(dict_rep["uuid"]):
+ uuid_owner = (
+ db.session.query(cls)
+ .filter(cls.uuid == dict_rep["uuid"])
+ .first()
+ )
+ if uuid_owner is not None and uuid_owner is not obj:
+ del dict_rep["uuid"]
+
if not obj:
is_new_obj = True
# Create new DB object
@@ -615,6 +707,7 @@ class ImportExportMixin(UUIDMixin):
recursive=recursive,
include_parent_ref=include_parent_ref,
include_defaults=include_defaults,
+ export_uuids=export_uuids,
)
for child in getattr(self, cld)
],
diff --git a/tests/integration_tests/databases/commands_tests.py b/tests/integration_tests/databases/commands_tests.py
index e45bbcfe034..734dd55a3f4 100644
--- a/tests/integration_tests/databases/commands_tests.py
+++ b/tests/integration_tests/databases/commands_tests.py
@@ -178,6 +178,20 @@ class TestExportDatabasesCommand(SupersetTestCase):
metadata = yaml.safe_load(contents["datasets/examples/birth_names.yaml"]())
metadata.pop("uuid")
+ # Datasets in a database bundle are exported with ``export_uuids=True``,
+ # so every column/metric carries a ``uuid`` so that custom folder
+ # references survive the round trip. They are assigned dynamically, so
+ # build lookups by name.
+ birth_names = next(
+ table for table in example_db.tables if table.table_name == "birth_names"
+ )
+ column_uuid_map = {
+ column.column_name: str(column.uuid) for column in birth_names.columns
+ }
+ metric_uuid_map = {
+ metric.metric_name: str(metric.uuid) for metric in birth_names.metrics
+ }
+
metadata["columns"].sort(key=lambda x: x["column_name"])
expected_metadata = {
"cache_timeout": None,
@@ -194,6 +208,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": ds_type,
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["ds"],
},
{
"column_name": "gender",
@@ -207,6 +222,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": "STRING" if example_db.backend == "hive" else "VARCHAR(16)",
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["gender"],
},
{
"column_name": "name",
@@ -222,6 +238,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
),
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["name"],
},
{
"column_name": "num",
@@ -235,6 +252,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": big_int_type,
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["num"],
},
{
"column_name": "num_california",
@@ -248,6 +266,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": None,
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["num_california"],
},
{
"column_name": "state",
@@ -261,6 +280,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": "STRING" if example_db.backend == "hive" else "VARCHAR(10)",
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["state"],
},
{
"column_name": "num_boys",
@@ -274,6 +294,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": big_int_type,
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["num_boys"],
},
{
"column_name": "num_girls",
@@ -287,6 +308,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"type": big_int_type,
"advanced_data_type": None,
"verbose_name": None,
+ "uuid": column_uuid_map["num_girls"],
},
],
"database_uuid": str(db_uuid),
@@ -306,6 +328,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"metric_type": "count",
"verbose_name": "COUNT(*)",
"warning_text": None,
+ "uuid": metric_uuid_map["count"],
},
{
"d3format": None,
@@ -316,6 +339,7 @@ class TestExportDatabasesCommand(SupersetTestCase):
"metric_type": None,
"verbose_name": None,
"warning_text": None,
+ "uuid": metric_uuid_map["sum__num"],
},
],
"offset": 0,
diff --git a/tests/integration_tests/datasets/commands_tests.py b/tests/integration_tests/datasets/commands_tests.py
index 54b448142cf..5edb01ca87b 100644
--- a/tests/integration_tests/datasets/commands_tests.py
+++ b/tests/integration_tests/datasets/commands_tests.py
@@ -95,6 +95,14 @@ class TestExportDatasetsCommand(SupersetTestCase):
type_map = {
column.column_name: str(column.type) for column in example_dataset.columns
}
+ # column/metric UUIDs are exported so folder references survive import;
+ # they are assigned dynamically, so build lookups by name.
+ column_uuid_map = {
+ column.column_name: str(column.uuid) for column in example_dataset.columns
+ }
+ metric_uuid_map = {
+ metric.metric_name: str(metric.uuid) for metric in example_dataset.metrics
+ }
assert metadata == {
"cache_timeout": None,
@@ -115,6 +123,7 @@ class TestExportDatasetsCommand(SupersetTestCase):
"advanced_data_type": None,
"verbose_name": None,
"extra": None,
+ "uuid": column_uuid_map["source"],
},
{
"column_name": "target",
@@ -127,6 +136,7 @@ class TestExportDatasetsCommand(SupersetTestCase):
"is_dttm": False,
"python_date_format": None,
"type": type_map["target"],
+ "uuid": column_uuid_map["target"],
"advanced_data_type": None,
"verbose_name": None,
"extra": None,
@@ -145,6 +155,7 @@ class TestExportDatasetsCommand(SupersetTestCase):
"advanced_data_type": None,
"verbose_name": None,
"extra": None,
+ "uuid": column_uuid_map["value"],
},
],
"database_uuid": str(example_db.uuid),
@@ -165,6 +176,7 @@ class TestExportDatasetsCommand(SupersetTestCase):
"metric_type": "count",
"verbose_name": "COUNT(*)",
"warning_text": None,
+ "uuid": metric_uuid_map["count"],
},
{
"currency": None,
@@ -176,6 +188,7 @@ class TestExportDatasetsCommand(SupersetTestCase):
"metric_type": None,
"verbose_name": None,
"warning_text": None,
+ "uuid": metric_uuid_map["sum__value"],
},
],
"folders": None,
diff --git a/tests/unit_tests/datasets/commands/export_test.py b/tests/unit_tests/datasets/commands/export_test.py
index d7cbc9dd37c..9656813daf8 100644
--- a/tests/unit_tests/datasets/commands/export_test.py
+++ b/tests/unit_tests/datasets/commands/export_test.py
@@ -41,10 +41,27 @@ def test_export(session: Session) -> None:
db.session.flush()
columns = [
- TableColumn(column_name="ds", is_dttm=1, type="TIMESTAMP"),
- TableColumn(column_name="user_id", type="INTEGER"),
- TableColumn(column_name="revenue", type="INTEGER"),
- TableColumn(column_name="expenses", type="INTEGER"),
+ TableColumn(
+ column_name="ds",
+ is_dttm=1,
+ type="TIMESTAMP",
+ uuid=UUID("00000000-0000-0000-0000-000000000006"),
+ ),
+ TableColumn(
+ column_name="user_id",
+ type="INTEGER",
+ uuid=UUID("00000000-0000-0000-0000-000000000007"),
+ ),
+ TableColumn(
+ column_name="revenue",
+ type="INTEGER",
+ uuid=UUID("00000000-0000-0000-0000-000000000008"),
+ ),
+ TableColumn(
+ column_name="expenses",
+ type="INTEGER",
+ uuid=UUID("00000000-0000-0000-0000-000000000009"),
+ ),
TableColumn(
column_name="profit",
type="INTEGER",
@@ -211,6 +228,7 @@ metrics:
extra:
warning_markdown: null
warning_text: null
+ uuid: 00000000-0000-0000-0000-000000000004
columns:
- column_name: profit
verbose_name: null
@@ -226,6 +244,7 @@ columns:
datetime_format: null
extra:
certified_by: User
+ uuid: 00000000-0000-0000-0000-000000000005
- column_name: ds
verbose_name: null
is_dttm: 1
@@ -239,6 +258,7 @@ columns:
python_date_format: null
datetime_format: null
extra: null
+ uuid: 00000000-0000-0000-0000-000000000006
- column_name: user_id
verbose_name: null
is_dttm: false
@@ -252,7 +272,8 @@ columns:
python_date_format: null
datetime_format: null
extra: null
-- column_name: expenses
+ uuid: 00000000-0000-0000-0000-000000000007
+- column_name: revenue
verbose_name: null
is_dttm: false
is_active: true
@@ -265,7 +286,8 @@ columns:
python_date_format: null
datetime_format: null
extra: null
-- column_name: revenue
+ uuid: 00000000-0000-0000-0000-000000000008
+- column_name: expenses
verbose_name: null
is_dttm: false
is_active: true
@@ -278,6 +300,7 @@ columns:
python_date_format: null
datetime_format: null
extra: null
+ uuid: 00000000-0000-0000-0000-000000000009
version: 1.0.0
database_uuid: {database.uuid}
""",
@@ -307,6 +330,55 @@ version: 1.0.0
]
+def test_export_database_bundle_includes_child_uuids(session: Session) -> None:
+ """
+ Datasets embedded in a *database* export must carry metric/column UUIDs.
+
+ ``ExportDatabasesCommand`` reaches datasets through its own
+ ``export_to_dict(recursive=True, export_uuids=True)`` call rather than
+ through ``ExportDatasetsCommand``, so it needs its own coverage: without
+ ``export_uuids`` being forwarded to the recursive child export, the
+ ``datasets/`` files in a database bundle would drop the UUIDs that custom
+ folder assignments reference.
+ """
+ from superset.commands.database.export import ExportDatabasesCommand
+ from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
+ from superset.models.core import Database
+
+ 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()
+
+ metric_uuid = UUID("00000000-0000-0000-0000-0000000000f1")
+ column_uuid = UUID("00000000-0000-0000-0000-0000000000f2")
+ sqla_table = SqlaTable(
+ table_name="my_table",
+ database=database,
+ metrics=[
+ SqlMetric(metric_name="cnt", expression="COUNT(*)", uuid=metric_uuid),
+ ],
+ columns=[
+ TableColumn(column_name="profit", type="INTEGER", uuid=column_uuid),
+ ],
+ )
+ db.session.add(sqla_table)
+ db.session.flush()
+
+ contents = {
+ path: content_fn()
+ for path, content_fn in ExportDatabasesCommand._export(database) # pylint: disable=protected-access
+ }
+ dataset_paths = [path for path in contents if path.startswith("datasets/")]
+ assert len(dataset_paths) == 1
+
+ payload = yaml.safe_load(contents[dataset_paths[0]])
+ assert [metric["uuid"] for metric in payload["metrics"]] == [str(metric_uuid)]
+ assert [column["uuid"] for column in payload["columns"]] == [str(column_uuid)]
+
+
def test_export_two_datasets_same_table_name_different_schema(
session: Session,
) -> None:
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 d568bde8e62..85c026050d8 100644
--- a/tests/unit_tests/datasets/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py
@@ -26,8 +26,10 @@ from unittest.mock import Mock, patch
from urllib import request
import pytest
+import yaml
from flask import current_app
from flask_appbuilder.security.sqla.models import Role, User
+from marshmallow import ValidationError
from pytest_mock import MockerFixture
from sqlalchemy.orm.session import Session
@@ -253,6 +255,451 @@ def test_import_dataset(mocker: MockerFixture, session: Session) -> None:
assert sqla_table.database.id == database.id
+def test_export_import_round_trip_preserves_metric_folder_membership(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ A metric (or column) assigned to a custom folder must stay in that folder
+ after the dataset is exported and imported into another workspace.
+
+ Folder leaves reference metrics/columns by UUID. If the export drops the
+ metric/column UUIDs, the importer recreates them with fresh random UUIDs
+ while the ``folders`` JSON still points at the originals — so the metric can
+ no longer be matched to its folder and is re-homed to the default folder.
+ This exercises the full export -> import round trip and asserts the
+ imported metric/column keep the UUIDs the folder leaves reference.
+ """
+ from superset.commands.dataset.export import ExportDatasetsCommand
+ from superset.connectors.sqla.models import SqlMetric
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
+
+ # --- source workspace: a dataset with a metric + column pinned to a custom
+ # folder, referenced by UUID ---
+ source_db = Database(database_name="source_db", sqlalchemy_uri="sqlite://")
+ db.session.add(source_db)
+ db.session.flush()
+
+ metric_uuid = uuid.UUID("00000000-0000-0000-0000-0000000000a1")
+ column_uuid = uuid.UUID("00000000-0000-0000-0000-0000000000b2")
+ folder_uuid = uuid.UUID("00000000-0000-0000-0000-0000000000c3")
+
+ sqla_table = SqlaTable(
+ table_name="my_table",
+ database=source_db,
+ columns=[
+ TableColumn(column_name="profit", type="INTEGER", uuid=column_uuid),
+ ],
+ metrics=[
+ SqlMetric(metric_name="cnt", expression="COUNT(*)", uuid=metric_uuid),
+ ],
+ folders=[
+ {
+ "uuid": str(folder_uuid),
+ "type": "folder",
+ "name": "Custom",
+ "children": [
+ {"uuid": str(metric_uuid), "type": "metric"},
+ {"uuid": str(column_uuid), "type": "column"},
+ ],
+ },
+ ],
+ )
+ db.session.add(sqla_table)
+ db.session.flush()
+
+ # --- export (command-level YAML payload) ---
+ config = yaml.safe_load(ExportDatasetsCommand._file_content(sqla_table)) # pylint: disable=protected-access
+
+ # The exported metric/column must carry their UUIDs so the folder
+ # references survive the round trip.
+ assert config["metrics"][0].get("uuid") == str(metric_uuid)
+ assert any(col.get("uuid") == str(column_uuid) for col in config["columns"])
+
+ # The import schema must accept and preserve those UUIDs; without the schema
+ # fields it would reject them as unknown and the round trip would break.
+ loaded = ImportV1DatasetSchema().load(config)
+ assert loaded["metrics"][0]["uuid"] == metric_uuid
+ assert any(col["uuid"] == column_uuid for col in loaded["columns"])
+
+ # --- import into another workspace ---
+ # Model a separate workspace: drop the source dataset so its UUIDs are free
+ # (a fresh workspace has never seen them), then import against a fresh
+ # database and a brand-new dataset UUID so the importer creates the
+ # metric/column anew from the exported payload.
+ db.session.delete(sqla_table)
+ db.session.flush()
+
+ target_db = Database(database_name="target_db", sqlalchemy_uri="sqlite://")
+ db.session.add(target_db)
+ db.session.flush()
+ config["database_id"] = target_db.id
+ config["uuid"] = str(uuid.uuid4())
+
+ imported = import_dataset(config)
+
+ # The metric/column must be recreated with their original UUIDs so the
+ # typed folder leaves still resolve to them — i.e. they stay in the custom
+ # folder rather than being re-homed to the default one.
+ imported_metric = next(m for m in imported.metrics if m.metric_name == "cnt")
+ assert imported_metric.uuid == metric_uuid
+ imported_column = next(c for c in imported.columns if c.column_name == "profit")
+ assert imported_column.uuid == column_uuid
+
+ # The folders JSON is stored verbatim (it round-trips with or without the
+ # fix); the uuid assertions above are the real gate that its leaves still
+ # resolve to the imported children. This just documents the expected shape.
+ assert imported.folders == [
+ {
+ "uuid": str(folder_uuid),
+ "type": "folder",
+ "name": "Custom",
+ "children": [
+ {"uuid": str(metric_uuid), "type": "metric"},
+ {"uuid": str(column_uuid), "type": "column"},
+ ],
+ },
+ ]
+
+
+def test_import_dataset_clone_with_duplicate_child_uuid_gets_fresh_uuid(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Cloning a dataset by importing its config under a new dataset UUID must not
+ fail when the original — and its metric/column UUIDs — still exist.
+
+ Metric/column UUIDs are globally unique but matched only within their parent
+ on import, so an unchanged child UUID under a *new* dataset would otherwise
+ violate the unique constraint on INSERT. The importer drops the colliding
+ UUID and lets a fresh one be assigned, so the clone imports cleanly (the
+ pre-uuid-export behavior for that workflow).
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ 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()
+
+ metric_uuid = "00000000-0000-0000-0000-0000000000d4"
+ column_uuid = "00000000-0000-0000-0000-0000000000e5"
+ config = {
+ "table_name": "my_table",
+ "uuid": str(uuid.uuid4()),
+ "metrics": [
+ {"metric_name": "cnt", "expression": "COUNT(*)", "uuid": metric_uuid},
+ ],
+ "columns": [
+ {"column_name": "profit", "type": "INTEGER", "uuid": column_uuid},
+ ],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+
+ original = import_dataset(copy.deepcopy(config))
+ assert str(original.metrics[0].uuid) == metric_uuid
+ assert str(original.columns[0].uuid) == column_uuid
+
+ # Clone: same child UUIDs, but a new dataset UUID and name (as a user editing
+ # the exported config to duplicate the dataset would produce).
+ clone_config = copy.deepcopy(config)
+ clone_config["table_name"] = "my_table_clone"
+ clone_config["uuid"] = str(uuid.uuid4())
+
+ clone = import_dataset(clone_config)
+
+ # The clone imports without hitting the unique constraint, and its children
+ # receive fresh UUIDs while the original keeps its own.
+ assert clone.id != original.id
+ assert [m.metric_name for m in clone.metrics] == ["cnt"]
+ assert [c.column_name for c in clone.columns] == ["profit"]
+ assert str(clone.metrics[0].uuid) != metric_uuid
+ assert str(clone.columns[0].uuid) != column_uuid
+ assert str(original.metrics[0].uuid) == metric_uuid
+ assert str(original.columns[0].uuid) == column_uuid
+
+
+def test_import_dataset_clone_overwrite_reimport_keeps_fresh_child_uuid(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Overwriting a cloned dataset with its own bundle must not resurrect the
+ original's child UUIDs.
+
+ On the overwrite path children sync with ``sync=["columns", "metrics"]`` and
+ are matched within the parent by name, so re-importing the clone bundle
+ (which still carries the original's metric/column UUIDs) would otherwise
+ ``setattr`` those UUIDs onto the clone's children and violate the global
+ unique constraint at flush. The importer drops any incoming child UUID owned
+ by a different row on the UPDATE branch too, so the overwrite succeeds and
+ the clone's children keep the fresh UUIDs assigned on first import.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ 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()
+
+ metric_uuid = "00000000-0000-0000-0000-0000000000d6"
+ column_uuid = "00000000-0000-0000-0000-0000000000e7"
+ config = {
+ "table_name": "my_table",
+ "uuid": str(uuid.uuid4()),
+ "metrics": [
+ {"metric_name": "cnt", "expression": "COUNT(*)", "uuid": metric_uuid},
+ ],
+ "columns": [
+ {"column_name": "profit", "type": "INTEGER", "uuid": column_uuid},
+ ],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+
+ original = import_dataset(copy.deepcopy(config))
+
+ # Clone under a new dataset UUID and name; its children get fresh UUIDs
+ # because the originals still exist.
+ clone_config = copy.deepcopy(config)
+ clone_config["table_name"] = "my_table_clone"
+ clone_config["uuid"] = str(uuid.uuid4())
+ clone = import_dataset(copy.deepcopy(clone_config))
+ fresh_metric_uuid = str(clone.metrics[0].uuid)
+ fresh_column_uuid = str(clone.columns[0].uuid)
+ assert fresh_metric_uuid != metric_uuid
+ assert fresh_column_uuid != column_uuid
+
+ # Re-import the same clone bundle with overwrite=True. The children match by
+ # (table_id, name), and the bundle still carries the original's UUIDs; the
+ # guard must keep this from writing them onto the clone's children.
+ reimported = import_dataset(copy.deepcopy(clone_config), overwrite=True)
+
+ assert reimported.id == clone.id
+ assert [m.metric_name for m in reimported.metrics] == ["cnt"]
+ assert [c.column_name for c in reimported.columns] == ["profit"]
+ # The clone keeps its fresh child UUIDs and the original is untouched.
+ assert str(reimported.metrics[0].uuid) == fresh_metric_uuid
+ assert str(reimported.columns[0].uuid) == fresh_column_uuid
+ assert str(original.metrics[0].uuid) == metric_uuid
+ assert str(original.columns[0].uuid) == column_uuid
+
+
+def test_import_dataset_null_child_uuid_keeps_existing(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ An explicit ``uuid: null`` must not wipe an existing child's UUID.
+
+ The child import schemas accept ``uuid=None``. Without the guard the
+ overwrite path would ``setattr`` that ``None`` onto the matched child and
+ persist a literal NULL — the column is nullable and ``unique`` permits
+ repeated NULLs, so it would fail silently and orphan every folder leaf
+ pointing at that child.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ 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()
+
+ metric_uuid = "00000000-0000-0000-0000-0000000000f8"
+ config: dict[str, Any] = {
+ "table_name": "my_table",
+ "uuid": str(uuid.uuid4()),
+ "metrics": [
+ {"metric_name": "cnt", "expression": "COUNT(*)", "uuid": metric_uuid},
+ ],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+ dataset = import_dataset(copy.deepcopy(config))
+ assert str(dataset.metrics[0].uuid) == metric_uuid
+
+ # Re-import the same bundle with the child uuid explicitly nulled.
+ nulled = copy.deepcopy(config)
+ nulled["metrics"][0]["uuid"] = None
+ reimported = import_dataset(nulled, overwrite=True)
+
+ assert reimported.id == dataset.id
+ assert reimported.metrics[0].uuid is not None
+ assert str(reimported.metrics[0].uuid) == metric_uuid
+
+
+def test_import_dataset_ambiguous_child_aborts_instead_of_half_applying(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ An ambiguous metric/column match must abort the import, not half-apply it.
+
+ Children are matched within their parent by name *or* UUID, so a payload
+ metric can match one existing metric by name and a *different* one by UUID.
+ That raises ``MultipleResultsFound`` from inside the child import — after
+ the dataset's own fields were already updated and after earlier siblings
+ were already imported. Swallowing it (the legacy dataset-level contract)
+ would report success over a partially-applied import, so the importer must
+ raise ``ImportFailedError`` and let the transaction roll everything back.
+ """
+ from superset.connectors.sqla.models import SqlMetric
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ 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()
+
+ a_uuid = "00000000-0000-0000-0000-0000000000a1"
+ cnt_uuid = "00000000-0000-0000-0000-0000000000a2"
+ dataset_uuid = str(uuid.uuid4())
+ config: dict[str, Any] = {
+ "table_name": "my_table",
+ "description": "as exported",
+ "uuid": dataset_uuid,
+ "metrics": [
+ {"metric_name": "a", "expression": "COUNT(*)", "uuid": a_uuid},
+ {"metric_name": "cnt", "expression": "COUNT(*)", "uuid": cnt_uuid},
+ ],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+ dataset = import_dataset(copy.deepcopy(config))
+
+ # Simulate the target instance drifting: ``cnt`` is renamed to ``cnt_old``
+ # (keeping its UUID) and a brand-new metric takes the name ``cnt``. The
+ # dataset itself is edited too.
+ renamed = next(m for m in dataset.metrics if m.metric_name == "cnt")
+ renamed.metric_name = "cnt_old"
+ new_cnt_uuid = str(uuid.uuid4())
+ dataset.metrics.append(
+ SqlMetric(metric_name="cnt", expression="COUNT(1)", uuid=new_cnt_uuid)
+ )
+ dataset.description = "edited in the UI"
+ db.session.flush()
+ db.session.commit()
+
+ # Re-import the *original* bundle: its ``cnt``/``cnt_uuid`` metric matches
+ # the new ``cnt`` by name and ``cnt_old`` by UUID.
+ with pytest.raises(ImportFailedError) as excinfo:
+ import_dataset(copy.deepcopy(config), overwrite=True)
+
+ assert "matches two different existing" in str(excinfo.value)
+ assert "my_table" in str(excinfo.value)
+
+ # Because it raised, the caller's transaction can undo everything; nothing
+ # from the aborted import is visible afterwards.
+ db.session.rollback()
+
+ reloaded = db.session.query(SqlaTable).filter_by(uuid=dataset_uuid).one()
+ # The parent's scalar fields were NOT overwritten by the aborted payload.
+ assert reloaded.description == "edited in the UI"
+ # No metric was added, renamed, or deleted by the aborted import.
+ assert sorted(m.metric_name for m in reloaded.metrics) == ["a", "cnt", "cnt_old"]
+ assert {m.metric_name: str(m.uuid) for m in reloaded.metrics} == {
+ "a": a_uuid,
+ "cnt_old": cnt_uuid,
+ "cnt": new_cnt_uuid,
+ }
+
+
+def _dataset_config_with_children(
+ metrics: list[dict[str, Any]],
+ columns: list[dict[str, Any]],
+) -> dict[str, Any]:
+ return {
+ "version": "1.0.0",
+ "table_name": "my_table",
+ "uuid": str(uuid.uuid4()),
+ "database_uuid": str(uuid.uuid4()),
+ "metrics": metrics,
+ "columns": columns,
+ }
+
+
+def test_import_dataset_schema_rejects_duplicate_metric_uuids() -> None:
+ """
+ Two metrics sharing a UUID must be rejected by the import schema.
+
+ UUIDs are globally unique, so such a payload cannot be imported faithfully:
+ the second metric would match the first one by UUID and overwrite it in
+ place, silently collapsing two metrics into one.
+ """
+ duplicate = str(uuid.uuid4())
+ config = _dataset_config_with_children(
+ metrics=[
+ {"metric_name": "cnt", "expression": "COUNT(*)", "uuid": duplicate},
+ {"metric_name": "cnt2", "expression": "COUNT(1)", "uuid": duplicate},
+ ],
+ columns=[],
+ )
+
+ with pytest.raises(ValidationError) as excinfo:
+ ImportV1DatasetSchema().load(config)
+
+ assert "metrics" in excinfo.value.messages
+ assert duplicate in str(excinfo.value.messages["metrics"])
+
+
+def test_import_dataset_schema_rejects_duplicate_column_uuids() -> None:
+ """
+ Two columns sharing a UUID must be rejected by the import schema.
+ """
+ duplicate = str(uuid.uuid4())
+ config = _dataset_config_with_children(
+ metrics=[],
+ columns=[
+ {"column_name": "profit", "type": "INTEGER", "uuid": duplicate},
+ {"column_name": "revenue", "type": "INTEGER", "uuid": duplicate},
+ ],
+ )
+
+ with pytest.raises(ValidationError) as excinfo:
+ ImportV1DatasetSchema().load(config)
+
+ assert "columns" in excinfo.value.messages
+ assert duplicate in str(excinfo.value.messages["columns"])
+
+
+def test_import_dataset_schema_allows_distinct_and_missing_child_uuids() -> None:
+ """
+ Distinct UUIDs — and children with no UUID at all — remain valid.
+
+ Only the ``null``/absent case may repeat: a pre-uuid-export bundle has no
+ child UUIDs whatsoever and must keep importing.
+ """
+ config = _dataset_config_with_children(
+ metrics=[
+ {"metric_name": "cnt", "expression": "COUNT(*)", "uuid": str(uuid.uuid4())},
+ {"metric_name": "cnt2", "expression": "COUNT(1)"},
+ {"metric_name": "cnt3", "expression": "COUNT(2)", "uuid": None},
+ ],
+ columns=[
+ {"column_name": "profit", "type": "INTEGER", "uuid": str(uuid.uuid4())},
+ {"column_name": "revenue", "type": "INTEGER"},
+ {"column_name": "expenses", "type": "INTEGER", "uuid": None},
+ ],
+ )
+
+ loaded = ImportV1DatasetSchema().load(config)
+
+ assert len(loaded["metrics"]) == 3
+ assert len(loaded["columns"]) == 3
+
+
def test_import_dataset_no_folder(mocker: MockerFixture, session: Session) -> None:
"""
Test importing a dataset that was exported without folders.