sha174n commented on PR #44080: URL: https://github.com/apache/superset/pull/44080#issuecomment-5877096487
@rusackas you're right, that one was a real gap and the port was incomplete. Fixed in ccb5395. `_repoints_sql` is now a sibling predicate alongside `_repoints_table`, porting `UpdateDatasetCommand._validate_sql_access`. It fires when the request supplies `sql` and that SQL would land somewhere new: different text than the stored value, a different database connection, or the same text with the catalog/schema it resolves unqualified names against moved. That last case was open in the first cut of the fix too, and it is the same hole on the command side. The two checks stay independent and both can run on one save, matching `_apply_database_repoint` plus `_validate_sql_access`. They need separate `raise_for_access` calls rather than one combined call: passing `sql` with `database` builds an ephemeral `Query` that supersedes `table`, so a single call would silently drop the table check. Three regression tests added (physical-to-virtual conversion, virtual SQL replaced, virtual schema moved) plus one for the combined cross-database case; each fails with the gate removed. Same commit folds the `database_changed` leg into `_repoints_table` so one function owns the whole predicate instead of half of it living at the call site, and restores the guard on the `database_id` write so an unchanged database is not marked dirty on the way to the 409. 29 tests green, pre-commit clean. -- 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]
