-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix restore from backup for definers #64595
Conversation
This is an automated comment for commit 74a9d55 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
Successful checks
|
@@ -1086,7 +1086,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) | |||
create.sql_security = std::make_shared<ASTSQLSecurity>(); | |||
|
|||
if (create.sql_security) | |||
processSQLSecurityOption(getContext(), create.sql_security->as<ASTSQLSecurity &>(), create.attach, create.is_materialized_view); | |||
processSQLSecurityOption(getContext(), create.sql_security->as<ASTSQLSecurity &>(), create.is_materialized_view, /* skip_check_permissions= */ is_restore_from_backup || create.attach); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use LoadingStrictnessLevel
instead of create.attach
, so it will work correctly for distributed DDL. Also, we can get rid of is_restore_from_backup
and replace it with SECONDARY_CREATE
Also, just re-running CI doesn't make any sense, especially when you ignore real bugs found by the CI:
|
This bug isn't related to the PR, so I restarted the CI several times just to be sure. |
Unrelated bugs are still worth attention. What's the point of running the CI if we ignore bugs that it finds? If you want to be sure, then please try to investigate the failures: you will see if they are related or not. You cannot be sure by just re-running the CI, because checks may be flaky, and re-run may hide a real bug if it's not 100% reproducible. |
The broken test is flaky and unrelated |
Backport #64595 to 24.5: Fix restore from backup for definers
Backport #64595 to 24.4: Fix restore from backup for definers
Co-authored-by: robot-clickhouse <[email protected]>
…s-restore Fix restore from backup for definers
…ners_restore_from_backup 24.3 Backport of ClickHouse#64595 - fix definers restore from backup
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix the crash loop when restoring from backup is blocked by creating an MV with a definer that hasn't been restored yet.
Documentation entry for user-facing changes
CI Settings
NOTE: If your merge the PR with modified CI you MUST KNOW what you are doing
NOTE: Checked options will be applied if set before CI RunConfig/PrepareRunConfig step