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 5d771e9f467 fix(dataset): apply the import overwrite permission check
to identity-matched datasets (#43058)
5d771e9f467 is described below
commit 5d771e9f467d73c03796e20749cde9ccb79392e2
Author: Shaitan <[email protected]>
AuthorDate: Wed Sep 9 05:37:14 2026 +0100
fix(dataset): apply the import overwrite permission check to
identity-matched datasets (#43058)
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
superset/commands/chart/importers/v1/__init__.py | 6 +-
.../commands/dashboard/importers/v1/__init__.py | 6 +-
superset/commands/dataset/importers/v1/utils.py | 46 +++---
superset/commands/importers/v1/assets.py | 6 +-
superset/commands/importers/v1/utils.py | 54 +++++++
superset/datasets/schemas.py | 8 +-
.../importers/v1/test_find_existing_for_import.py | 134 ++++++++++++++++-
.../datasets/commands/importers/v1/import_test.py | 165 +++++++++++++++++++++
tests/unit_tests/datasets/schema_tests.py | 20 +++
9 files changed, 421 insertions(+), 24 deletions(-)
diff --git a/superset/commands/chart/importers/v1/__init__.py
b/superset/commands/chart/importers/v1/__init__.py
index 3f0ea68608a..6340c713f4e 100644
--- a/superset/commands/chart/importers/v1/__init__.py
+++ b/superset/commands/chart/importers/v1/__init__.py
@@ -87,7 +87,11 @@ class ImportChartsCommand(ImportModelsCommand):
):
config["database_id"] = database_ids[config["database_uuid"]]
dataset = import_dataset(config, overwrite=False)
- datasets[str(dataset.uuid)] = dataset
+ # Key on the bundle's own uuid, which is what the bundle's
+ # charts reference. An import that resolves onto an existing
+ # dataset by physical identity returns a row whose uuid
+ # differs, and keying on that would strand those charts.
+ datasets[str(config["uuid"])] = dataset
# Resolve the creator's default viewers once for the whole bundle
# rather than once per chart (a membership query each).
diff --git a/superset/commands/dashboard/importers/v1/__init__.py
b/superset/commands/dashboard/importers/v1/__init__.py
index 787ea74a8e0..22084581f73 100644
--- a/superset/commands/dashboard/importers/v1/__init__.py
+++ b/superset/commands/dashboard/importers/v1/__init__.py
@@ -158,7 +158,11 @@ class ImportDashboardsCommand(ImportModelsCommand):
):
config["database_id"] = database_ids[config["database_uuid"]]
dataset = import_dataset(config, overwrite=overwrite_assets)
- dataset_info[str(dataset.uuid)] = {
+ # Key on the bundle's own uuid, which is what the bundle's
+ # charts reference. An import that resolves onto an existing
+ # dataset by physical identity returns a row whose uuid
+ # differs, and keying on that would strand those charts.
+ dataset_info[str(config["uuid"])] = {
"datasource_id": dataset.id,
"datasource_type": dataset.datasource_type,
"datasource_name": dataset.table_name,
diff --git a/superset/commands/dataset/importers/v1/utils.py
b/superset/commands/dataset/importers/v1/utils.py
index 39dd81b476c..cd41d3ec812 100644
--- a/superset/commands/dataset/importers/v1/utils.py
+++ b/superset/commands/dataset/importers/v1/utils.py
@@ -41,7 +41,10 @@ from superset.commands.dataset.exceptions import (
MultiCatalogDisabledValidationError,
)
from superset.commands.exceptions import ImportFailedError
-from superset.commands.importers.v1.utils import find_existing_for_import
+from superset.commands.importers.v1.utils import (
+ find_existing_by_import_identity,
+ find_existing_for_import,
+)
from superset.connectors.sqla.models import SqlaTable
from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES
from superset.daos.dataset import DatasetDAO
@@ -293,7 +296,15 @@ def import_dataset( # noqa: C901
# implicit-restore re-import is a clean replacement, not a merge.
is_soft_deleted_match = False
- if existing := find_existing_for_import(SqlaTable, config["uuid"]):
+ existing = find_existing_for_import(SqlaTable, config["uuid"])
+ if not existing and can_write:
+ # A fresh UUID over the (database, catalog, schema, table) identity of
+ # an existing dataset is still matched-and-updated by
+ # ``import_from_dict``, so resolving only the UUID would leave that
+ # update ungated. Resolve the identity the same way the import will.
+ # (Soft-deleted twins are handled in the create branch further down.)
+ existing = find_existing_by_import_identity(SqlaTable, config)
+ if existing:
if existing.deleted_at is not None:
# RESTORE path — re-importing a soft-deleted UUID is an implicit
# restore-with-update, a distinct operation from overwriting an
@@ -520,10 +531,8 @@ def import_dataset( # noqa: C901
# raise so the operator can resolve the legacy-NULL-schema
# ambiguity before re-uploading.
if is_soft_deleted_match:
- # ``is_soft_deleted_match`` is only ever set inside the
- # ``if existing := ...`` walrus block, so ``existing`` is
- # guaranteed non-None here. The assert pins the invariant
- # for mypy.
+ # Set only inside the ``if existing:`` block above; the assert
+ # pins that invariant for mypy.
assert existing is not None
existing.deleted_at = original_deleted_at
db.session.flush()
@@ -534,17 +543,20 @@ def import_dataset( # noqa: C901
"manually before retrying."
) from ex
# On the non-soft-deleted overwrite path the legacy contract
- # holds: return the existing row unmodified. Bypasses the
- # visibility filter so a soft-deleted duplicate can be located
- # too — without the bypass the listener would hide the row and
- # the ``.one()`` would raise NoResultFound, masking the
- # original MultipleResultsFound.
- dataset = (
- db.session.query(SqlaTable)
- .execution_options(**{SKIP_VISIBILITY_FILTER_CLASSES: {SqlaTable}})
- .filter_by(uuid=config["uuid"])
- .one()
- )
+ # holds: return the existing row unmodified. Prefer the row already
+ # resolved above — on an identity match the incoming uuid belongs to
+ # no row at all, so looking it up again would raise NoResultFound and
+ # mask the original MultipleResultsFound. Falling back to the uuid
+ # lookup bypasses the visibility filter so a soft-deleted duplicate
+ # can still be located, for the same reason.
+ dataset = existing
+ if dataset is None:
+ dataset = (
+ db.session.query(SqlaTable)
+ .execution_options(**{SKIP_VISIBILITY_FILTER_CLASSES:
{SqlaTable}})
+ .filter_by(uuid=config["uuid"])
+ .one()
+ )
if dataset.id is None:
db.session.flush()
diff --git a/superset/commands/importers/v1/assets.py
b/superset/commands/importers/v1/assets.py
index 25cd33c7d25..c0e92a85d2f 100644
--- a/superset/commands/importers/v1/assets.py
+++ b/superset/commands/importers/v1/assets.py
@@ -135,7 +135,11 @@ class ImportAssetsCommand(BaseCommand):
if file_name.startswith("datasets/"):
config["database_id"] = database_ids[config["database_uuid"]]
dataset = import_dataset(config, overwrite=overwrite)
- dataset_info[str(dataset.uuid)] = {
+ # Key on the bundle's own uuid, which is what the bundle's
+ # charts reference. An import that resolves onto an existing
+ # dataset by physical identity returns a row whose uuid
+ # differs, and keying on that would strand those charts.
+ dataset_info[str(config["uuid"])] = {
"datasource_id": dataset.id,
"datasource_type": dataset.datasource_type,
"datasource_name": dataset.table_name,
diff --git a/superset/commands/importers/v1/utils.py
b/superset/commands/importers/v1/utils.py
index b8272dfafde..b88f1e9f4a0 100644
--- a/superset/commands/importers/v1/utils.py
+++ b/superset/commands/importers/v1/utils.py
@@ -22,6 +22,7 @@ import yaml
from flask import current_app
from marshmallow import fields, Schema, validate
from marshmallow.exceptions import ValidationError
+from sqlalchemy import and_, or_
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.orm import Session
@@ -589,6 +590,59 @@ def find_existing_for_import(model_cls: type[Any], uuid:
str) -> Any | None:
)
+def find_existing_by_import_identity(
+ model_cls: type[Any], config: dict[str, Any]
+) -> Any | None:
+ """Look up an active row by the identity an import would bind it to.
+
+ ``ImportExportMixin.import_from_dict`` does not match on ``uuid`` alone: it
+ matches on a disjunction of *every* unique constraint the model declares,
+ dropping the keys the incoming config leaves null. A config carrying a
+ fresh ``uuid`` can therefore still be matched-and-updated onto an existing
+ row through one of those other constraints.
+
+ A caller that gates an overwrite on a ``uuid`` hit alone (see
+ :func:`find_existing_for_import`) would miss that row and let the import
+ update it ungated, so this resolves the same identity the import will,
+ using the model's own constraint metadata rather than a copy of it that can
+ drift. Returns ``None`` for models whose only unique key is ``uuid``.
+
+ Soft-deleted rows are excluded: they are invisible to
+ ``import_from_dict`` too, so they cannot be reached this way. Callers that
+ need them use :meth:`DatasetDAO.find_soft_deleted_logical_duplicate` and
+ friends, which apply the *semantic* identity rules (default-catalog
+ normalization) rather than mirroring the import lookup.
+
+ Results are ordered by primary key: not every logical unique constraint is
+ enforced physically (see ``SqlaTable.__table_args__``), so duplicates can
+ exist and the gate must pick the same row every time.
+ """
+ # pylint: disable=protected-access
+ predicates = []
+ for columns in model_cls._unique_constraints():
+ if "uuid" in columns:
+ continue
+ # Mirror ``import_from_dict``: a key the config leaves null drops out
of
+ # the predicate instead of being matched as ``IS NULL``.
+ terms = [
+ getattr(model_cls, column) == config[column]
+ for column in sorted(columns)
+ if config.get(column) is not None
+ ]
+ if terms:
+ predicates.append(and_(*terms))
+
+ if not predicates:
+ return None
+
+ return (
+ db.session.query(model_cls)
+ .filter(or_(*predicates))
+ .order_by(model_cls.id)
+ .first()
+ )
+
+
def clear_soft_deleted_for_import(existing: Any) -> None:
"""Hard-delete a soft-deleted row to free its UUID for re-import.
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index a6f30532855..c7beb350c16 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -333,8 +333,12 @@ class ImportV1ColumnSchema(Schema):
filterable = fields.Boolean()
expression = fields.String(allow_none=True)
description = fields.String(allow_none=True)
- python_date_format = fields.String(allow_none=True)
- datetime_format = fields.String(allow_none=True)
+ python_date_format = fields.String(
+ allow_none=True, validate=[Length(1, 255), validate_python_date_format]
+ )
+ datetime_format = fields.String(
+ allow_none=True, validate=[Length(1, 100), validate_python_date_format]
+ )
uuid = fields.UUID(allow_none=True)
diff --git
a/tests/unit_tests/commands/importers/v1/test_find_existing_for_import.py
b/tests/unit_tests/commands/importers/v1/test_find_existing_for_import.py
index e15e5f85af3..f75040ccc51 100644
--- a/tests/unit_tests/commands/importers/v1/test_find_existing_for_import.py
+++ b/tests/unit_tests/commands/importers/v1/test_find_existing_for_import.py
@@ -26,18 +26,20 @@ as-is so the importer can decide overwrite/permission before
from __future__ import annotations
from collections.abc import Generator
+from typing import Any
import pytest
-from sqlalchemy import Column, Integer, String
+from sqlalchemy import Column, Integer, String, UniqueConstraint
from sqlalchemy.orm import declarative_base
from sqlalchemy.orm.session import Session
from sqlalchemy_utils import UUIDType
from superset.commands.importers.v1.utils import (
clear_soft_deleted_for_import,
+ find_existing_by_import_identity,
find_existing_for_import,
)
-from superset.models.helpers import SoftDeleteMixin
+from superset.models.helpers import ImportExportMixin, SoftDeleteMixin
_TestBase = declarative_base()
@@ -49,6 +51,40 @@ class _ImportableSoftDeletable(SoftDeleteMixin, _TestBase):
# type: ignore[misc
uuid = Column(UUIDType(binary=False), unique=True)
+class _ImportableWithIdentity(ImportExportMixin, _TestBase): # type:
ignore[misc, valid-type]
+ """Stands in for a model with a unique key other than ``uuid``."""
+
+ __tablename__ = "_importable_with_identity_test"
+ __table_args__ = (UniqueConstraint("parent_id", "namespace", "name"),)
+ id = Column(Integer, primary_key=True)
+ parent_id = Column(Integer, nullable=False)
+ namespace = Column(String, nullable=True)
+ name = Column(String, nullable=False)
+ uuid = Column(UUIDType(binary=False), unique=True)
+
+
+class _ImportableUUIDOnly(ImportExportMixin, _TestBase): # type: ignore[misc,
valid-type]
+ """Stands in for a model whose only unique key is ``uuid``."""
+
+ __tablename__ = "_importable_uuid_only_test"
+ # Every model reaching import_from_dict declares __table_args__;
+ # _unique_constraints reads it directly.
+ __table_args__: tuple[Any, ...] = ()
+ id = Column(Integer, primary_key=True)
+ name = Column(String, nullable=False)
+ uuid = Column(UUIDType(binary=False), unique=True)
+
+
[email protected]
+def _identity_tables(session: Session) -> Generator[None, None, None]:
+ bind = session.get_bind()
+ _ImportableWithIdentity.__table__.create(bind)
+ _ImportableUUIDOnly.__table__.create(bind)
+ yield
+ _ImportableWithIdentity.__table__.drop(bind)
+ _ImportableUUIDOnly.__table__.drop(bind)
+
+
@pytest.fixture
def _synthetic_table(session: Session) -> Generator[None, None, None]:
_ImportableSoftDeletable.metadata.create_all(session.get_bind())
@@ -178,3 +214,97 @@ def test_find_then_clear_is_the_intended_caller_sequence(
session.flush()
assert fresh.id is not None
assert fresh.deleted_at is None
+
+
[email protected]("_identity_tables")
+def test_find_by_identity_matches_non_uuid_unique_key(
+ app_context: None, session: Session
+) -> None:
+ """A fresh uuid still resolves to the row owning the config's identity."""
+ import uuid as uuid_lib
+
+ row = _ImportableWithIdentity(
+ parent_id=1, namespace="finance", name="salaries",
uuid=uuid_lib.uuid4()
+ )
+ session.add(row)
+ session.flush()
+
+ config = {
+ "parent_id": 1,
+ "namespace": "finance",
+ "name": "salaries",
+ "uuid": str(uuid_lib.uuid4()),
+ }
+ assert find_existing_by_import_identity(_ImportableWithIdentity, config)
is row
+
+
[email protected]("_identity_tables")
+def test_find_by_identity_drops_null_config_keys(
+ app_context: None, session: Session
+) -> None:
+ """``import_from_dict`` drops null keys from its predicate, so a config
+ that omits ``namespace`` still reaches a row that stores one. Matching it
+ with an exact ``IS NULL`` instead would miss that row."""
+ import uuid as uuid_lib
+
+ row = _ImportableWithIdentity(
+ parent_id=1, namespace="finance", name="salaries",
uuid=uuid_lib.uuid4()
+ )
+ session.add(row)
+ session.flush()
+
+ config = {"parent_id": 1, "name": "salaries", "uuid":
str(uuid_lib.uuid4())}
+ assert find_existing_by_import_identity(_ImportableWithIdentity, config)
is row
+
+
[email protected]("_identity_tables")
+def test_find_by_identity_returns_none_without_match(
+ app_context: None, session: Session
+) -> None:
+ import uuid as uuid_lib
+
+ session.add(
+ _ImportableWithIdentity(
+ parent_id=1, namespace="finance", name="salaries",
uuid=uuid_lib.uuid4()
+ )
+ )
+ session.flush()
+
+ config = {"parent_id": 2, "name": "salaries", "uuid":
str(uuid_lib.uuid4())}
+ assert find_existing_by_import_identity(_ImportableWithIdentity, config)
is None
+
+
[email protected]("_identity_tables")
+def test_find_by_identity_is_deterministic_across_duplicates(
+ app_context: None, session: Session
+) -> None:
+ """Not every logical unique constraint is enforced physically, so the gate
+ must pick the same row on every run."""
+ import uuid as uuid_lib
+
+ first, second = (
+ _ImportableWithIdentity(
+ parent_id=1, namespace=namespace, name="salaries",
uuid=uuid_lib.uuid4()
+ )
+ for namespace in ("finance", "hr")
+ )
+ session.add_all([first, second])
+ session.flush()
+
+ config = {"parent_id": 1, "name": "salaries", "uuid":
str(uuid_lib.uuid4())}
+ assert find_existing_by_import_identity(_ImportableWithIdentity, config)
is first
+
+
[email protected]("_identity_tables")
+def test_find_by_identity_returns_none_for_uuid_only_model(
+ app_context: None, session: Session
+) -> None:
+ """With no unique key besides ``uuid`` there is no alternate identity to
+ resolve, so the helper must not fall back to matching everything."""
+ import uuid as uuid_lib
+
+ session.add(_ImportableUUIDOnly(name="whatever", uuid=uuid_lib.uuid4()))
+ session.flush()
+
+ config = {"name": "whatever", "uuid": str(uuid_lib.uuid4())}
+ assert find_existing_by_import_identity(_ImportableUUIDOnly, config) is
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 bebc5687527..d4943ea4df6 100644
--- a/tests/unit_tests/datasets/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py
@@ -2383,6 +2383,171 @@ def
test_import_restore_blocked_by_active_twin_at_incoming_identity(
assert existing.deleted_at is not None
[email protected]("config_catalog", ["public", None])
+def test_import_dataset_identity_collision_requires_overwrite_permission(
+ mocker: MockerFixture, session: Session, config_catalog: str | None
+) -> None:
+ """
+ A config with a fresh UUID but the physical identity of an existing ACTIVE
+ dataset must go through the same overwrite permission gate as a UUID match.
+
+ The ``None`` case matters on its own: ``import_from_dict`` drops null keys
+ from its uniqueness predicate, so a catalog-less config still reaches a
+ dataset stored under a catalog.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ mocker.patch.object(security_manager, "is_editor", return_value=False)
+ mocker.patch.object(security_manager, "is_admin", return_value=False)
+
+ 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()
+
+ victim = SqlaTable(
+ table_name="salaries",
+ schema="finance",
+ catalog="public",
+ database_id=database.id,
+ uuid="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+ sql="SELECT * FROM finance.salaries",
+ )
+ db.session.add(victim)
+ db.session.flush()
+
+ importer_user = User(
+ username="importer",
+ first_name="at",
+ last_name="tacker",
+ email="[email protected]",
+ )
+
+ # Fresh UUID, but the same physical identity as ``victim``.
+ config = {
+ "table_name": "salaries",
+ "schema": "finance",
+ "catalog": config_catalog,
+ "sql": "SELECT * FROM finance.salaries -- clobbered",
+ "uuid": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+ "metrics": [],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+
+ with override_user(importer_user):
+ with pytest.raises(ImportFailedError) as excinfo:
+ import_dataset(copy.deepcopy(config), overwrite=True)
+ assert "overwrite" in str(excinfo.value).lower()
+
+ # The victim dataset must not have been clobbered.
+ assert victim.sql == "SELECT * FROM finance.salaries"
+
+
+def test_import_dataset_identity_collision_overwrites_in_place_for_editor(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Once the gate passes, an identity collision updates the existing dataset in
+ place rather than creating a twin, and the caller's config is left alone so
+ a bundle importer that re-reads or retries it still sees its own UUID.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ mocker.patch.object(security_manager, "is_editor", 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()
+
+ existing = SqlaTable(
+ table_name="salaries",
+ schema="finance",
+ catalog="public",
+ database_id=database.id,
+ uuid="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+ sql="SELECT * FROM finance.salaries",
+ )
+ db.session.add(existing)
+ db.session.flush()
+
+ config = {
+ "table_name": "salaries",
+ "schema": "finance",
+ "catalog": "public",
+ "sql": "SELECT * FROM finance.salaries -- updated",
+ "uuid": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+ "metrics": [],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+
+ imported = import_dataset(config, overwrite=True)
+
+ assert imported.id == existing.id
+ assert imported.sql == "SELECT * FROM finance.salaries -- updated"
+ assert db.session.query(SqlaTable).count() == 1
+ assert config["uuid"] == "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
+
+
+def
test_import_dataset_identity_collision_with_duplicate_rows_returns_existing(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ A config that omits the catalog matches every row sharing its (database,
+ schema, table), so it can be ambiguous. ``import_from_dict`` then raises
+ ``MultipleResultsFound`` and the legacy fallback returns the existing row
+ unmodified. It must not look the incoming UUID up again: on an identity
+ match that UUID belongs to no row, and the lookup would raise.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ mocker.patch.object(security_manager, "is_editor", 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()
+
+ for dataset_uuid, catalog in (
+ ("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", "public"),
+ ("cccccccc-cccc-cccc-cccc-cccccccccccc", "private"),
+ ):
+ db.session.add(
+ SqlaTable(
+ table_name="salaries",
+ schema="finance",
+ catalog=catalog,
+ database_id=database.id,
+ uuid=dataset_uuid,
+ sql="SELECT * FROM finance.salaries",
+ )
+ )
+ db.session.flush()
+
+ config = {
+ "table_name": "salaries",
+ "schema": "finance",
+ "sql": "SELECT * FROM finance.salaries -- updated",
+ "uuid": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+ "metrics": [],
+ "columns": [],
+ "database_uuid": database.uuid,
+ "database_id": database.id,
+ }
+
+ dataset = import_dataset(copy.deepcopy(config), overwrite=True)
+
+ assert str(dataset.uuid) == "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+ assert dataset.sql == "SELECT * FROM finance.salaries"
+
+
def test_peer_validating_connection_blocks_rebound_peer() -> None:
"""
The import fetch validates the connected peer address, so a hostname that
diff --git a/tests/unit_tests/datasets/schema_tests.py
b/tests/unit_tests/datasets/schema_tests.py
index 23c493de64f..713c09ea80c 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -296,3 +296,23 @@ def test_import_v1_metric_schema_parses_currency_string()
-> None:
}
result = schema.load(data)
assert result["currency"] == {"symbol": "CAD", "symbolPosition": "suffix"}
+
+
+def test_import_v1_column_schema_validates_date_formats() -> None:
+ """ImportV1ColumnSchema validates python_date_format/datetime_format the
+ same way the edit (PUT) schema does, so the import path does not accept a
+ format string the edit path would reject."""
+ from superset.datasets.schemas import ImportV1ColumnSchema
+
+ schema = ImportV1ColumnSchema()
+
+ ok = schema.load(
+ {"column_name": "ds", "python_date_format": "%Y-%m-%d", "is_dttm":
True}
+ )
+ assert ok["python_date_format"] == "%Y-%m-%d"
+
+ with pytest.raises(ValidationError):
+ schema.load({"column_name": "ds", "python_date_format": "%Y-%m-%d'"})
+
+ with pytest.raises(ValidationError):
+ schema.load({"column_name": "ds", "datetime_format": "not a format'"})
