sha174n commented on code in PR #43058:
URL: https://github.com/apache/superset/pull/43058#discussion_r3962679791
##########
superset/commands/dataset/importers/v1/utils.py:
##########
@@ -248,7 +248,30 @@ 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 config carrying a fresh UUID but a (database, catalog, schema,
+ # table) identity that already belongs to an active dataset would be
+ # matched-and-updated on that unique key by ``import_from_dict``,
+ # without passing through the overwrite gate below (which only ran on a
+ # UUID match). Resolve that collision to the existing dataset here so
+ # the same gate applies; align the UUID so the subsequent import
updates
+ # that row deterministically. (Soft-deleted twins are handled in the
+ # create branch further down.)
+ active_twin = (
+ db.session.query(SqlaTable)
+ .filter_by(
+ database_id=config["database_id"],
+ catalog=config.get("catalog"),
+ schema=config.get("schema"),
+ table_name=config["table_name"],
+ )
+ .first()
Review Comment:
Fixed in d1cc301. The probe now goes through a shared
`find_existing_by_import_identity` helper that derives its predicate from the
model unique-constraint metadata and drops keys the config leaves null, exactly
as `import_from_dict` does, so a catalog-less config still resolves a dataset
stored under a catalog. Covered by a parametrized regression test.
##########
superset/commands/dataset/importers/v1/utils.py:
##########
@@ -248,7 +248,30 @@ 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 config carrying a fresh UUID but a (database, catalog, schema,
+ # table) identity that already belongs to an active dataset would be
+ # matched-and-updated on that unique key by ``import_from_dict``,
+ # without passing through the overwrite gate below (which only ran on a
+ # UUID match). Resolve that collision to the existing dataset here so
+ # the same gate applies; align the UUID so the subsequent import
updates
+ # that row deterministically. (Soft-deleted twins are handled in the
+ # create branch further down.)
+ active_twin = (
+ db.session.query(SqlaTable)
+ .filter_by(
+ database_id=config["database_id"],
+ catalog=config.get("catalog"),
+ schema=config.get("schema"),
+ table_name=config["table_name"],
+ )
+ .first()
+ )
+ if active_twin is not None:
+ config["uuid"] = str(active_twin.uuid)
+ existing = find_existing_for_import(SqlaTable, config["uuid"])
Review Comment:
Fixed in d1cc301. The UUID rewrite is gone entirely rather than copied
around: `import_from_dict` already resolves the row by that identity, so
aligning the UUID was redundant. The caller config is left untouched, with a
test asserting it keeps its own UUID after a collision resolves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
