mikebridge commented on PR #40130:
URL: https://github.com/apache/superset/pull/40130#issuecomment-4614228752
Thanks @richardfogaca — addressed in 7096621f4b (and a small mypy follow-up
in b65c253078).
**\`utils.py:255\` — restore-via-import + \`MultipleResultsFound\`
fallback**: you're right, the prior flow cleared \`deleted_at\` and flushed
before \`SqlaTable.import_from_dict\` had a chance to apply the upload. If
\`import_from_dict\` then raised \`MultipleResultsFound\` (legacy NULL-schema
ambiguity at line 303), the fallback returned the row unmodified — meaning the
operator got a "restore" they didn't ask for, with the upload contents silently
dropped.
Fix: snapshot \`existing.deleted_at\` before the clear, then check
\`is_soft_deleted_match\` in the \`except MultipleResultsFound\` handler. If
both conditions hold, restore the snapshotted \`deleted_at\` value, flush, and
raise \`ImportFailedError\` with a message explaining the legacy-ambiguity
cause:
\`\`\`python
if is_soft_deleted_match:
existing.deleted_at = original_deleted_at
db.session.flush()
raise ImportFailedError(
"Soft-deleted dataset has an ambiguous legacy duplicate "
"(likely from the NULL-schema migration). Cannot restore-"
"and-update via re-import; resolve the duplicate manually "
"before retrying."
) from None
\`\`\`
The non-soft-deleted overwrite path keeps the legacy "return unmodified"
contract from the original 2018 fix — that semantic still applies when the
operator is doing an explicit overwrite, just not when soft-delete-restore is
in play.
Same commit also addresses a bito follow-up: the docstring on
\`test_create_blocked_by_soft_deleted_logical_duplicate\` referenced the
deleted \`test_restore_blocked_by_active_logical_duplicate\` (removed in
c4560c1b39 as unreachable due to the DB-level unique constraint). Rewrote the
docstring to describe what the test actually pins: \`validate_uniqueness\`
bypasses the visibility filter so a soft-deleted twin blocks creates at the
application layer with a clean 422.
Thanks for the precise file:line trace through the flush ordering — the
\`utils.py:228\` snapshot anchor made the fix shape obvious.
--
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]
