(superset) 04/15: fix(datasets): count an absent key in the echo exemption as unchanged

[hu...@apache.org]

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 3ee2fe0c7c3b8a39516116107b47ca0af99bd39b
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Wed Oct 14 13:30:05 2026 +0000

    fix(datasets): count an absent key in the echo exemption as unchanged
    
    `_upsert_columns` writes only the keys a column entry carries, so a payload
    echoing the transform without the monotonicity declaration -- or the
    declaration without the transform -- is a valid partial update. The
    exemption compared the missing key as `None`, read that as a change, and
    wrote *both* fields back as null: the same unrecoverable loss the exemption
    was added to prevent, reached by the shape it was added for.
    
    Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
    Signed-off-by: Hugh A. Miles II <hu...@gmail.com>
---
 superset/connectors/sqla/partition_mapping.py      | 17 ++++++++++--
 .../connectors/sqla/partition_mapping_test.py      | 32 ++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/superset/connectors/sqla/partition_mapping.py b/superset/connectors/sqla/partition_mapping.py
index 01c828242b9..a87d2489c67 100644
--- a/superset/connectors/sqla/partition_mapping.py
+++ b/superset/connectors/sqla/partition_mapping.py
@@ -2242,9 +2242,20 @@ def drop_unmapped_value_transforms(
             continue
         if stored_by_name is not None and name in stored_by_name:
             stored_transform, stored_monotonic = stored_by_name[name]
-            if transform == stored_transform and bool(is_monotonic) == bool(
-                stored_monotonic
-            ):
+            # A key the entry does not carry is unchanged, not `False`:
+            # `_upsert_columns` writes only the keys present, so echoing the
+            # transform without the declaration is a valid partial update. Read
+            # as a change, it failed the exemption and wrote both fields back
+            # as null -- the loss the exemption exists to prevent.
+            monotonic_echoes = (
+                "partition_transform_is_monotonic" not in column
+                or bool(is_monotonic) == bool(stored_monotonic)
+            )
+            transform_echoes = (
+                "partition_value_transform" not in column
+                or transform == stored_transform
+            )
+            if transform_echoes and monotonic_echoes:
                 # An echo, not an introduction. See `stored_by_name`.
                 continue
         column["partition_value_transform"] = None
diff --git a/tests/unit_tests/connectors/sqla/partition_mapping_test.py b/tests/unit_tests/connectors/sqla/partition_mapping_test.py
index d5c2facd943..aca0ee5a28d 100644
--- a/tests/unit_tests/connectors/sqla/partition_mapping_test.py
+++ b/tests/unit_tests/connectors/sqla/partition_mapping_test.py
@@ -3119,6 +3119,38 @@ def test_a_changed_transform_on_an_unmapped_column_is_still_dropped(
     assert columns[0]["partition_value_transform"] is None
 
 
+@pytest.mark.parametrize(
+    "entry",
+    [
+        # The transform echoed, the declaration simply absent.
+        {"id": 7, "partition_value_transform": "unix_timestamp(:value)"},
+        # And the reverse: the declaration echoed, the transform absent.
+        {"id": 7, "partition_transform_is_monotonic": True},
+    ],
+)
+def test_a_partial_echo_counts_as_unchanged(entry: dict[str, Any]) -> None:
+    """
+    `_upsert_columns` writes only the keys an entry carries, so a payload
+    echoing one of the two fields is a valid partial update. Read as a change,
+    it failed the exemption and both fields were written back as null -- the
+    same unrecoverable loss the exemption exists to prevent.
+    """
+    columns = [dict(entry)]
+
+    assert (
+        drop_unmapped_value_transforms(
+            columns,
+            partition_column="dt_epoch",
+            partition_mapped_column=None,
+            main_dttm_col="event_time2",
+            names_by_id={7: "event_time"},
+            stored_by_name={"event_time": ("unix_timestamp(:value)", True)},
+        )
+        == []
+    )
+    assert columns[0] == entry
+
+
 def test_a_caller_that_cannot_see_storage_keeps_the_stricter_behaviour() -> None:
     """
     `stored_by_name` is optional, and omitting it drops an unmapped column's