You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2026/10/13 18:15:24 UTC
(superset) 04/07: fix(datasets): give the v0 import the same mapping repairs as v1
This is an automated email from the ASF dual-hosted git repository.
hughhhh pushed a commit to branch hughhhh/pfm-6-explore-indicator
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5330d7688a6297ed2541923cac45cfbfe3717959
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Tue Oct 13 16:14:49 2026 +0000
fix(datasets): give the v0 import the same mapping repairs as v1
The repair added to this path handled a dangling reference, which is only
one of the three things the v1 importer normalizes.
An *explicit* self-mapping -- `partition_mapped_column` equal to
`partition_column` -- is blocking in `UpdateDatasetCommand`, so a legacy
bundle setting both to the same existing column imported cleanly and then
failed every later edit, including a description-only PUT. Cleared here the
way v1 clears it: only the override, which leaves the mapping following
`main_dttm_col`.
And an omitted monotonicity declaration kept whatever was stored, because
this path applies the raw config with no schema default behind it. A bundle
supplying `abs(:value)` without the key inherited a `True` from the
transform it replaced, which enables range mirrors that drop rows the
filter keeps: `n >= -1` becomes `p >= 1` and misses `n = 0, p = 0`.
`_default_omitted_monotonic_declarations` is reused from v1 rather than
restated, wrapped in a walk because a v0 bundle nests datasets under
`databases[].tables[]` when it came from the CLI and lists them bare when
it came from the UI.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
Signed-off-by: Hugh A. Miles II <hu...@gmail.com>
---
superset/commands/dataset/importers/v0.py | 52 +++++++
.../datasets/commands/importers/v0/import_test.py | 164 +++++++++++++++++++++
2 files changed, 216 insertions(+)
diff --git a/superset/commands/dataset/importers/v0.py b/superset/commands/dataset/importers/v0.py
index eb5b02967bc..65cc925d4c5 100644
--- a/superset/commands/dataset/importers/v0.py
+++ b/superset/commands/dataset/importers/v0.py
@@ -26,6 +26,9 @@ from superset.commands.base import BaseCommand
from superset.commands.database.exceptions import DatabaseNotFoundError
from superset.commands.dataset.exceptions import DatasetInvalidError
from superset.commands.exceptions import ImportFailedError
+from superset.commands.dataset.importers.v1.utils import (
+ _default_omitted_monotonic_declarations,
+)
from superset.commands.importers.exceptions import IncorrectVersionError
from superset.connectors.sqla.models import (
BaseDatasource,
@@ -289,6 +292,7 @@ class ImportDatasetsCommand(BaseCommand):
imported: list[SqlaTable] = []
for file_name, config in self._configs.items():
logger.info("Importing dataset from file %s", file_name)
+ self._default_omitted_monotonic_declarations(config)
if isinstance(config, dict):
imported.extend(import_from_dict(config, sync=self.sync))
else: # list
@@ -308,6 +312,36 @@ class ImportDatasetsCommand(BaseCommand):
self._repair_partition_mappings(imported)
+ @classmethod
+ def _default_omitted_monotonic_declarations(cls, config: Any) -> None:
+ """
+ Materialize `partition_transform_is_monotonic` wherever a transform is
+ set, anywhere in a v0 config.
+
+ The v1 importer does this on its own raw config, and for the same
+ reason: an overwrite keeps whatever the bundle omits, including a
+ ``True`` left behind by the transform this bundle is replacing. A
+ non-monotonic transform declared monotonic enables range mirrors that
+ exclude rows the filter keeps -- ``abs(:value)`` inheriting a stored
+ ``True`` turns ``n >= -1`` into ``p >= 1`` and misses ``n = 0``.
+
+ Walked rather than addressed directly because a v0 bundle comes in two
+ shapes: a CLI export nests datasets under ``databases[].tables[]``,
+ while a UI export is a bare list of datasets. Only columns that supply
+ a transform are touched, so a re-import that says nothing about a
+ column stays idempotent.
+ """
+ if isinstance(config, list):
+ for entry in config:
+ cls._default_omitted_monotonic_declarations(entry)
+ return
+ if not isinstance(config, dict):
+ return
+ _default_omitted_monotonic_declarations(config.get("columns") or [])
+ for key in (DATABASES_KEY, "tables"):
+ for child in config.get(key) or []:
+ cls._default_omitted_monotonic_declarations(child)
+
@staticmethod
def _repair_partition_mappings(datasets: list[SqlaTable]) -> None:
"""
@@ -338,6 +372,24 @@ class ImportDatasetsCommand(BaseCommand):
DatasetDAO.clear_dangling_partition_mapping(
dataset, {column.column_name for column in dataset.columns}
)
+ # The other reference a bundle can leave unsaveable, cleared the way
+ # the v1 importer clears it: an *explicit* self-mapping is blocking
+ # in `UpdateDatasetCommand`, so a bundle setting both references to
+ # the same existing column imported happily and then failed every
+ # later edit, including a description-only PUT. Dropping only the
+ # override leaves the mapping following `main_dttm_col`, which is
+ # the state a bundle omitting the field would have produced.
+ if (
+ dataset.partition_mapped_column
+ and dataset.partition_mapped_column == dataset.partition_column
+ ):
+ logger.warning(
+ "Clearing the partition mapped column on dataset %s during "
+ "import: it names the partition column itself, which no "
+ "later save would accept",
+ dataset.table_name,
+ )
+ dataset.partition_mapped_column = None
DatasetDAO.clear_unmapped_partition_transforms(dataset)
def validate(self) -> None:
diff --git a/tests/unit_tests/datasets/commands/importers/v0/import_test.py b/tests/unit_tests/datasets/commands/importers/v0/import_test.py
index 02c2cb8fdb5..c279b4f87c3 100644
--- a/tests/unit_tests/datasets/commands/importers/v0/import_test.py
+++ b/tests/unit_tests/datasets/commands/importers/v0/import_test.py
@@ -166,3 +166,167 @@ def test_a_v0_database_bundle_repairs_its_datasets_too(mocker: MockerFixture) ->
dangling.assert_called_once_with(dataset, {"ds"})
unmapped.assert_called_once_with(dataset)
+
+
+def test_a_v0_import_clears_an_explicit_self_mapping(mocker: MockerFixture) -> None:
+ """
+ A dangling reference is not the only mapping a later save refuses: an
+ explicit self-mapping -- `partition_mapped_column` equal to
+ `partition_column` -- is blocking in `UpdateDatasetCommand`. A legacy
+ bundle setting both to the same existing column imported cleanly and then
+ failed every later edit, including a description-only PUT. The v1 importer
+ already clears it; this path did not.
+ """
+ from superset.commands.dataset.importers import v0
+ from superset.daos.dataset import DatasetDAO
+
+ dataset = mocker.MagicMock()
+ dataset.columns = [mocker.MagicMock(column_name="event_time")]
+ dataset.partition_column = "event_time"
+ dataset.partition_mapped_column = "event_time"
+ mocker.patch.object(v0.SqlaTable, "import_from_dict", return_value=dataset)
+ mocker.patch.object(v0.db.session, "flush")
+ mocker.patch.object(
+ v0.db.session, "query", return_value=mocker.MagicMock()
+ ).return_value.filter_by.return_value.one.return_value = mocker.MagicMock(id=1)
+ mocker.patch.object(DatasetDAO, "clear_dangling_partition_mapping")
+ mocker.patch.object(DatasetDAO, "clear_unmapped_partition_transforms")
+
+ command = v0.ImportDatasetsCommand({})
+ command._configs = { # noqa: SLF001
+ "ds.yaml": [{"params": '{"database_name": "sample_db"}'}]
+ }
+ command.run()
+
+ # Only the override goes, which leaves the mapping following
+ # `main_dttm_col` -- the state a bundle omitting the field would produce.
+ assert dataset.partition_mapped_column is None
+ assert dataset.partition_column == "event_time"
+
+
+@pytest.mark.parametrize(
+ "config",
+ [
+ # A UI export: a bare list of datasets.
+ [
+ {
+ "params": '{"database_name": "sample_db"}',
+ "columns": [
+ {"column_name": "n", "partition_value_transform": "abs(:value)"}
+ ],
+ }
+ ],
+ # A CLI export: datasets nested under their database.
+ {
+ "databases": [
+ {
+ "database_name": "sample_db",
+ "tables": [
+ {
+ "table_name": "t",
+ "columns": [
+ {
+ "column_name": "n",
+ "partition_value_transform": "abs(:value)",
+ }
+ ],
+ }
+ ],
+ }
+ ]
+ },
+ ],
+ ids=["ui-export", "cli-export"],
+)
+def test_a_v0_import_defaults_an_omitted_monotonic_declaration(config: object) -> None:
+ """
+ An overwrite keeps whatever the bundle omits, including a `True` left
+ behind by the transform this bundle replaces. `abs(:value)` inheriting it
+ enables range mirrors that exclude rows the filter keeps: `n >= -1` becomes
+ `p >= 1` and misses `n = 0, p = 0`.
+
+ Both bundle shapes, because a v0 config nests datasets differently
+ depending on where it came from.
+ """
+ from superset.commands.dataset.importers import v0
+
+ v0.ImportDatasetsCommand._default_omitted_monotonic_declarations( # noqa: SLF001
+ config
+ )
+
+ columns = (
+ config[0]["columns"]
+ if isinstance(config, list)
+ else config["databases"][0]["tables"][0]["columns"]
+ )
+ assert columns[0]["partition_transform_is_monotonic"] is False
+
+
+def test_a_v0_import_leaves_a_transformless_column_alone() -> None:
+ """
+ Only columns that supply a transform are touched, so a re-import that says
+ nothing about a column stays idempotent -- and an explicit declaration the
+ bundle does carry is not overwritten.
+ """
+ from superset.commands.dataset.importers import v0
+
+ config = [
+ {
+ "columns": [
+ {"column_name": "plain"},
+ {
+ "column_name": "declared",
+ "partition_value_transform": "unix_timestamp(:value)",
+ "partition_transform_is_monotonic": True,
+ },
+ ]
+ }
+ ]
+
+ v0.ImportDatasetsCommand._default_omitted_monotonic_declarations( # noqa: SLF001
+ config
+ )
+
+ assert "partition_transform_is_monotonic" not in config[0]["columns"][0]
+ assert config[0]["columns"][1]["partition_transform_is_monotonic"] is True
+
+
+def test_a_v0_import_normalizes_before_it_writes(mocker: MockerFixture) -> None:
+ """
+ The wiring, not the helper: the tests above call
+ `_default_omitted_monotonic_declarations` directly, so deleting its call
+ from `run()` would leave them green while the default never reached
+ storage. Asserted on the payload `import_from_dict` actually receives.
+ """
+ from superset.commands.dataset.importers import v0
+ from superset.daos.dataset import DatasetDAO
+
+ dataset = mocker.MagicMock()
+ dataset.columns = []
+ dataset.partition_column = None
+ dataset.partition_mapped_column = None
+ imported = mocker.patch.object(
+ v0.SqlaTable, "import_from_dict", return_value=dataset
+ )
+ mocker.patch.object(v0.db.session, "flush")
+ mocker.patch.object(
+ v0.db.session, "query", return_value=mocker.MagicMock()
+ ).return_value.filter_by.return_value.one.return_value = mocker.MagicMock(id=1)
+ mocker.patch.object(DatasetDAO, "clear_dangling_partition_mapping")
+ mocker.patch.object(DatasetDAO, "clear_unmapped_partition_transforms")
+
+ command = v0.ImportDatasetsCommand({})
+ command._configs = { # noqa: SLF001
+ "ds.yaml": [
+ {
+ "params": '{"database_name": "sample_db"}',
+ "columns": [
+ {"column_name": "n", "partition_value_transform": "abs(:value)"}
+ ],
+ }
+ ]
+ }
+ command.run()
+
+ written = imported.call_args.args[0]
+ assert written["columns"][0]["partition_transform_is_monotonic"] is False