-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Restore external tables to Null supports no settings #69253
Conversation
This is an automated comment for commit ccc6390 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
|
@@ -1074,6 +1074,7 @@ namespace | |||
engine_ast->name = "Null"; | |||
engine_ast->no_empty_args = true; | |||
storage.set(storage.engine, engine_ast); | |||
storage.settings = nullptr; |
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.
That is the actual fix.
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.
Do we also need to update/deal with other parameters such as partition_by
, primary_key
, etc? If so maybe it'd be simpler to use forEachPointerToChild
and set them all to NULL, then set the engine. WDYT?
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.
I can try to add a test and check.
…null-settings-fix
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Restore mode that replaces all external table engines and functions to Null (
restore_replace_external_engines_to_null
,restore_replace_external_table_functions_to_null
settings) was failing if table had SETTINGS. Now it removes settings from table definition in this case and allows to restore such tables.Documentation entry for user-facing changes
CI Settings (Only check the boxes if you know what you are doing):