mikebridge commented on PR #42797: URL: https://github.com/apache/superset/pull/42797#issuecomment-5194647443
The main `(id, uuid)` fix is correct, but one significant shadow-row read remains id-only. **HIGH — `superset/versioning/baseline/collection.py:126`: baseline detection still identifies entities by reusable ID alone.** `shadow_row_count()` filters only on `version_table.c.id == obj.id`. If predecessor shadow rows survive, its ID is reused, and the successor was created while capture was disabled, the first save after capture is enabled sees the predecessor history and skips creating the successor baseline. This is the same identity-collision class this PR is intended to eliminate. Apply the `(id, uuid)` identity predicate here and add a regression test for create/version predecessor → hard-delete → create successor with capture disabled → enable capture → edit successor. **MEDIUM — `superset/versioning/factory.py:295`: the new no-op detection fix has no behavioral regression test.** The current fixture creates the successor while capture is enabled and never updates it, so it does not exercise `_matches_previous_version()`. The transition test above should also prove the predecessor cannot cause a genuine successor edit to be classified as a no-op. The API history, snapshot, ETag transaction, and restore queries are correctly pinned. Focused tests and pre-commit passed locally. The CodeAnt snapshot-query finding is resolved at HEAD; its concurrency concern is real but pre-existing rather than introduced here. Verdict: request changes until the baseline probe and transition regression are covered. -- 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]
