From 2dfd5b14db6768e19d1ac1d5b45eac67c94678ad Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 20 Aug 2021 20:59:57 +0300 Subject: [PATCH 01/13] alter --- src/Parsers/ASTAlterQuery.cpp | 19 +- src/Parsers/ASTAlterQuery.h | 10 +- src/Parsers/ParserAlterQuery.cpp | 1147 +++++++++++++++--------------- src/Parsers/ParserAlterQuery.h | 11 +- 4 files changed, 615 insertions(+), 572 deletions(-) diff --git a/src/Parsers/ASTAlterQuery.cpp b/src/Parsers/ASTAlterQuery.cpp index d4424a60ffc..0e617ca7c21 100644 --- a/src/Parsers/ASTAlterQuery.cpp +++ b/src/Parsers/ASTAlterQuery.cpp @@ -461,11 +461,22 @@ void ASTAlterQuery::formatQueryImpl(const FormatSettings & settings, FormatState frame.need_parens = false; std::string indent_str = settings.one_line ? "" : std::string(4u * frame.indent, ' '); + settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str; - if (is_live_view) - settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << "ALTER LIVE VIEW " << (settings.hilite ? hilite_none : ""); - else - settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << "ALTER TABLE " << (settings.hilite ? hilite_none : ""); + switch (alter_object) + { + case AlterObjectType::TABLE: + settings.ostr << "ALTER TABLE "; + break; + case AlterObjectType::DATABASE: + settings.ostr << "ALTER DATABASE "; + break; + case AlterObjectType::LIVE_VIEW: + settings.ostr << "ALTER LIVE VIEW "; + break; + } + + settings.ostr << (settings.hilite ? hilite_none : ""); if (!table.empty()) { diff --git a/src/Parsers/ASTAlterQuery.h b/src/Parsers/ASTAlterQuery.h index 0fd6d2805ea..dadba107ddc 100644 --- a/src/Parsers/ASTAlterQuery.h +++ b/src/Parsers/ASTAlterQuery.h @@ -208,7 +208,15 @@ protected: class ASTAlterQuery : public ASTQueryWithTableAndOutput, public ASTQueryWithOnCluster { public: - bool is_live_view{false}; /// true for ALTER LIVE VIEW + enum class AlterObjectType + { + TABLE, + DATABASE, + LIVE_VIEW + }; + + // bool is_live_view{false}; /// true for ALTER LIVE VIEW + AlterObjectType alter_object = AlterObjectType::TABLE; ASTExpressionList * command_list = nullptr; diff --git a/src/Parsers/ParserAlterQuery.cpp b/src/Parsers/ParserAlterQuery.cpp index b282f276762..cb1796a70b5 100644 --- a/src/Parsers/ParserAlterQuery.cpp +++ b/src/Parsers/ParserAlterQuery.cpp @@ -123,494 +123,40 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected ParserSelectWithUnionQuery select_p; ParserTTLExpressionList parser_ttl_list; - if (is_live_view) + switch (alter_object) { - if (s_refresh.ignore(pos, expected)) + case ASTAlterQuery::AlterObjectType::LIVE_VIEW: { - command->type = ASTAlterCommand::LIVE_VIEW_REFRESH; - } - else - return false; - } - else - { - if (s_add_column.ignore(pos, expected)) - { - if (s_if_not_exists.ignore(pos, expected)) - command->if_not_exists = true; - - if (!parser_col_decl.parse(pos, command->col_decl, expected)) - return false; - - if (s_first.ignore(pos, expected)) - command->first = true; - else if (s_after.ignore(pos, expected)) + if (s_refresh.ignore(pos, expected)) { - if (!parser_name.parse(pos, command->column, expected)) - return false; - } - - command->type = ASTAlterCommand::ADD_COLUMN; - } - else if (s_rename_column.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->column, expected)) - return false; - - if (!s_to.ignore(pos, expected)) - return false; - - if (!parser_name.parse(pos, command->rename_to, expected)) - return false; - - command->type = ASTAlterCommand::RENAME_COLUMN; - } - else if (s_drop_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::DROP_PARTITION; - } - else if (s_drop_part.ignore(pos, expected)) - { - if (!parser_string_literal.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::DROP_PARTITION; - command->part = true; - } - else if (s_drop_detached_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::DROP_DETACHED_PARTITION; - } - else if (s_drop_detached_part.ignore(pos, expected)) - { - if (!parser_string_literal.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::DROP_DETACHED_PARTITION; - command->part = true; - } - else if (s_drop_column.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->column, expected)) - return false; - - command->type = ASTAlterCommand::DROP_COLUMN; - command->detach = false; - } - else if (s_clear_column.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->column, expected)) - return false; - - command->type = ASTAlterCommand::DROP_COLUMN; - command->clear_column = true; - command->detach = false; - - if (s_in_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - } - } - else if (s_add_index.ignore(pos, expected)) - { - if (s_if_not_exists.ignore(pos, expected)) - command->if_not_exists = true; - - if (!parser_idx_decl.parse(pos, command->index_decl, expected)) - return false; - - if (s_first.ignore(pos, expected)) - command->first = true; - else if (s_after.ignore(pos, expected)) - { - if (!parser_name.parse(pos, command->index, expected)) - return false; - } - - command->type = ASTAlterCommand::ADD_INDEX; - } - else if (s_drop_index.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->index, expected)) - return false; - - command->type = ASTAlterCommand::DROP_INDEX; - command->detach = false; - } - else if (s_clear_index.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->index, expected)) - return false; - - command->type = ASTAlterCommand::DROP_INDEX; - command->clear_index = true; - command->detach = false; - - if (s_in_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - } - } - else if (s_materialize_index.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->index, expected)) - return false; - - command->type = ASTAlterCommand::MATERIALIZE_INDEX; - command->detach = false; - - if (s_in_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - } - } - else if (s_add_projection.ignore(pos, expected)) - { - if (s_if_not_exists.ignore(pos, expected)) - command->if_not_exists = true; - - if (!parser_projection_decl.parse(pos, command->projection_decl, expected)) - return false; - - if (s_first.ignore(pos, expected)) - command->first = true; - else if (s_after.ignore(pos, expected)) - { - if (!parser_name.parse(pos, command->projection, expected)) - return false; - } - - command->type = ASTAlterCommand::ADD_PROJECTION; - } - else if (s_drop_projection.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->projection, expected)) - return false; - - command->type = ASTAlterCommand::DROP_PROJECTION; - command->detach = false; - } - else if (s_clear_projection.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->projection, expected)) - return false; - - command->type = ASTAlterCommand::DROP_PROJECTION; - command->clear_projection = true; - command->detach = false; - - if (s_in_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - } - } - else if (s_materialize_projection.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->projection, expected)) - return false; - - command->type = ASTAlterCommand::MATERIALIZE_PROJECTION; - command->detach = false; - - if (s_in_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - } - } - else if (s_move_part.ignore(pos, expected)) - { - if (!parser_string_literal.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::MOVE_PARTITION; - command->part = true; - - if (s_to_disk.ignore(pos)) - command->move_destination_type = DataDestinationType::DISK; - else if (s_to_volume.ignore(pos)) - command->move_destination_type = DataDestinationType::VOLUME; - else if (s_to_table.ignore(pos)) - { - if (!parseDatabaseAndTableName(pos, expected, command->to_database, command->to_table)) - return false; - command->move_destination_type = DataDestinationType::TABLE; - } - else if (s_to_shard.ignore(pos)) - { - command->move_destination_type = DataDestinationType::SHARD; + command->type = ASTAlterCommand::LIVE_VIEW_REFRESH; } else return false; - - if (command->move_destination_type != DataDestinationType::TABLE) - { - ASTPtr ast_space_name; - if (!parser_string_literal.parse(pos, ast_space_name, expected)) - return false; - - command->move_destination_name = ast_space_name->as().value.get(); - } + break; } - else if (s_move_partition.ignore(pos, expected)) + case ASTAlterQuery::AlterObjectType::DATABASE: { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::MOVE_PARTITION; - - if (s_to_disk.ignore(pos)) - command->move_destination_type = DataDestinationType::DISK; - else if (s_to_volume.ignore(pos)) - command->move_destination_type = DataDestinationType::VOLUME; - else if (s_to_table.ignore(pos)) + if (s_modify_setting.ignore(pos, expected)) { - if (!parseDatabaseAndTableName(pos, expected, command->to_database, command->to_table)) + if (!parser_settings.parse(pos, command->settings_changes, expected)) return false; - command->move_destination_type = DataDestinationType::TABLE; + command->type = ASTAlterCommand::MODIFY_SETTING; } else return false; - - if (command->move_destination_type != DataDestinationType::TABLE) + break; + } + case ASTAlterQuery::AlterObjectType::TABLE: + { + if (s_add_column.ignore(pos, expected)) { - ASTPtr ast_space_name; - if (!parser_string_literal.parse(pos, ast_space_name, expected)) + if (s_if_not_exists.ignore(pos, expected)) + command->if_not_exists = true; + + if (!parser_col_decl.parse(pos, command->col_decl, expected)) return false; - command->move_destination_name = ast_space_name->as().value.get(); - } - } - else if (s_add_constraint.ignore(pos, expected)) - { - if (s_if_not_exists.ignore(pos, expected)) - command->if_not_exists = true; - - if (!parser_constraint_decl.parse(pos, command->constraint_decl, expected)) - return false; - - command->type = ASTAlterCommand::ADD_CONSTRAINT; - } - else if (s_drop_constraint.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_name.parse(pos, command->constraint, expected)) - return false; - - command->type = ASTAlterCommand::DROP_CONSTRAINT; - command->detach = false; - } - else if (s_detach_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::DROP_PARTITION; - command->detach = true; - } - else if (s_detach_part.ignore(pos, expected)) - { - if (!parser_string_literal.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::DROP_PARTITION; - command->part = true; - command->detach = true; - } - else if (s_attach_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - if (s_from.ignore(pos)) - { - if (!parseDatabaseAndTableName(pos, expected, command->from_database, command->from_table)) - return false; - - command->replace = false; - command->type = ASTAlterCommand::REPLACE_PARTITION; - } - else - { - command->type = ASTAlterCommand::ATTACH_PARTITION; - } - } - else if (s_replace_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - if (!s_from.ignore(pos, expected)) - return false; - - if (!parseDatabaseAndTableName(pos, expected, command->from_database, command->from_table)) - return false; - - command->replace = true; - command->type = ASTAlterCommand::REPLACE_PARTITION; - } - else if (s_attach_part.ignore(pos, expected)) - { - if (!parser_string_literal.parse(pos, command->partition, expected)) - return false; - - command->part = true; - command->type = ASTAlterCommand::ATTACH_PARTITION; - } - else if (s_fetch_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - if (!s_from.ignore(pos, expected)) - return false; - - ASTPtr ast_from; - if (!parser_string_literal.parse(pos, ast_from, expected)) - return false; - - command->from = ast_from->as().value.get(); - command->type = ASTAlterCommand::FETCH_PARTITION; - } - else if (s_fetch_part.ignore(pos, expected)) - { - if (!parser_string_literal.parse(pos, command->partition, expected)) - return false; - - if (!s_from.ignore(pos, expected)) - return false; - - ASTPtr ast_from; - if (!parser_string_literal.parse(pos, ast_from, expected)) - return false; - command->from = ast_from->as().value.get(); - command->part = true; - command->type = ASTAlterCommand::FETCH_PARTITION; - } - else if (s_freeze.ignore(pos, expected)) - { - if (s_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::FREEZE_PARTITION; - } - else - { - command->type = ASTAlterCommand::FREEZE_ALL; - } - - /// WITH NAME 'name' - place local backup to directory with specified name - if (s_with.ignore(pos, expected)) - { - if (!s_name.ignore(pos, expected)) - return false; - - ASTPtr ast_with_name; - if (!parser_string_literal.parse(pos, ast_with_name, expected)) - return false; - - command->with_name = ast_with_name->as().value.get(); - } - } - else if (s_unfreeze.ignore(pos, expected)) - { - if (s_partition.ignore(pos, expected)) - { - if (!parser_partition.parse(pos, command->partition, expected)) - return false; - - command->type = ASTAlterCommand::UNFREEZE_PARTITION; - } - else - { - command->type = ASTAlterCommand::UNFREEZE_ALL; - } - - /// WITH NAME 'name' - remove local backup to directory with specified name - if (s_with.ignore(pos, expected)) - { - if (!s_name.ignore(pos, expected)) - return false; - - ASTPtr ast_with_name; - if (!parser_string_literal.parse(pos, ast_with_name, expected)) - return false; - - command->with_name = ast_with_name->as().value.get(); - } - else - { - return false; - } - } - else if (s_modify_column.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; - - if (!parser_modify_col_decl.parse(pos, command->col_decl, expected)) - return false; - - if (s_remove.ignore(pos, expected)) - { - if (s_default.ignore(pos, expected)) - command->remove_property = "DEFAULT"; - else if (s_materialized.ignore(pos, expected)) - command->remove_property = "MATERIALIZED"; - else if (s_alias.ignore(pos, expected)) - command->remove_property = "ALIAS"; - else if (s_comment.ignore(pos, expected)) - command->remove_property = "COMMENT"; - else if (s_codec.ignore(pos, expected)) - command->remove_property = "CODEC"; - else if (s_ttl.ignore(pos, expected)) - command->remove_property = "TTL"; - else - return false; - } - else - { if (s_first.ignore(pos, expected)) command->first = true; else if (s_after.ignore(pos, expected)) @@ -618,111 +164,581 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected if (!parser_name.parse(pos, command->column, expected)) return false; } + + command->type = ASTAlterCommand::ADD_COLUMN; } - command->type = ASTAlterCommand::MODIFY_COLUMN; - } - else if (s_modify_order_by.ignore(pos, expected)) - { - if (!parser_exp_elem.parse(pos, command->order_by, expected)) - return false; + else if (s_rename_column.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; - command->type = ASTAlterCommand::MODIFY_ORDER_BY; - } - else if (s_modify_sample_by.ignore(pos, expected)) - { - if (!parser_exp_elem.parse(pos, command->sample_by, expected)) - return false; + if (!parser_name.parse(pos, command->column, expected)) + return false; - command->type = ASTAlterCommand::MODIFY_SAMPLE_BY; - } - else if (s_delete.ignore(pos, expected)) - { - if (s_in_partition.ignore(pos, expected)) + if (!s_to.ignore(pos, expected)) + return false; + + if (!parser_name.parse(pos, command->rename_to, expected)) + return false; + + command->type = ASTAlterCommand::RENAME_COLUMN; + } + else if (s_drop_partition.ignore(pos, expected)) { if (!parser_partition.parse(pos, command->partition, expected)) return false; + + command->type = ASTAlterCommand::DROP_PARTITION; } + else if (s_drop_part.ignore(pos, expected)) + { + if (!parser_string_literal.parse(pos, command->partition, expected)) + return false; - if (!s_where.ignore(pos, expected)) - return false; - - if (!parser_exp_elem.parse(pos, command->predicate, expected)) - return false; - - command->type = ASTAlterCommand::DELETE; - } - else if (s_update.ignore(pos, expected)) - { - if (!parser_assignment_list.parse(pos, command->update_assignments, expected)) - return false; - - if (s_in_partition.ignore(pos, expected)) + command->type = ASTAlterCommand::DROP_PARTITION; + command->part = true; + } + else if (s_drop_detached_partition.ignore(pos, expected)) { if (!parser_partition.parse(pos, command->partition, expected)) return false; + + command->type = ASTAlterCommand::DROP_DETACHED_PARTITION; } + else if (s_drop_detached_part.ignore(pos, expected)) + { + if (!parser_string_literal.parse(pos, command->partition, expected)) + return false; - if (!s_where.ignore(pos, expected)) - return false; + command->type = ASTAlterCommand::DROP_DETACHED_PARTITION; + command->part = true; + } + else if (s_drop_column.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; - if (!parser_exp_elem.parse(pos, command->predicate, expected)) - return false; + if (!parser_name.parse(pos, command->column, expected)) + return false; - command->type = ASTAlterCommand::UPDATE; - } - else if (s_comment_column.ignore(pos, expected)) - { - if (s_if_exists.ignore(pos, expected)) - command->if_exists = true; + command->type = ASTAlterCommand::DROP_COLUMN; + command->detach = false; + } + else if (s_clear_column.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; - if (!parser_name.parse(pos, command->column, expected)) - return false; + if (!parser_name.parse(pos, command->column, expected)) + return false; - if (!parser_string_literal.parse(pos, command->comment, expected)) - return false; + command->type = ASTAlterCommand::DROP_COLUMN; + command->clear_column = true; + command->detach = false; - command->type = ASTAlterCommand::COMMENT_COLUMN; - } - else if (s_modify_ttl.ignore(pos, expected)) - { - if (!parser_ttl_list.parse(pos, command->ttl, expected)) - return false; - command->type = ASTAlterCommand::MODIFY_TTL; - } - else if (s_remove_ttl.ignore(pos, expected)) - { - command->type = ASTAlterCommand::REMOVE_TTL; - } - else if (s_materialize_ttl.ignore(pos, expected)) - { - command->type = ASTAlterCommand::MATERIALIZE_TTL; + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } + else if (s_add_index.ignore(pos, expected)) + { + if (s_if_not_exists.ignore(pos, expected)) + command->if_not_exists = true; - if (s_in_partition.ignore(pos, expected)) + if (!parser_idx_decl.parse(pos, command->index_decl, expected)) + return false; + + if (s_first.ignore(pos, expected)) + command->first = true; + else if (s_after.ignore(pos, expected)) + { + if (!parser_name.parse(pos, command->index, expected)) + return false; + } + + command->type = ASTAlterCommand::ADD_INDEX; + } + else if (s_drop_index.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->index, expected)) + return false; + + command->type = ASTAlterCommand::DROP_INDEX; + command->detach = false; + } + else if (s_clear_index.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->index, expected)) + return false; + + command->type = ASTAlterCommand::DROP_INDEX; + command->clear_index = true; + command->detach = false; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } + else if (s_materialize_index.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->index, expected)) + return false; + + command->type = ASTAlterCommand::MATERIALIZE_INDEX; + command->detach = false; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } + else if (s_add_projection.ignore(pos, expected)) + { + if (s_if_not_exists.ignore(pos, expected)) + command->if_not_exists = true; + + if (!parser_projection_decl.parse(pos, command->projection_decl, expected)) + return false; + + if (s_first.ignore(pos, expected)) + command->first = true; + else if (s_after.ignore(pos, expected)) + { + if (!parser_name.parse(pos, command->projection, expected)) + return false; + } + + command->type = ASTAlterCommand::ADD_PROJECTION; + } + else if (s_drop_projection.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->projection, expected)) + return false; + + command->type = ASTAlterCommand::DROP_PROJECTION; + command->detach = false; + } + else if (s_clear_projection.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->projection, expected)) + return false; + + command->type = ASTAlterCommand::DROP_PROJECTION; + command->clear_projection = true; + command->detach = false; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } + else if (s_materialize_projection.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->projection, expected)) + return false; + + command->type = ASTAlterCommand::MATERIALIZE_PROJECTION; + command->detach = false; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } + else if (s_move_part.ignore(pos, expected)) + { + if (!parser_string_literal.parse(pos, command->partition, expected)) + return false; + + command->type = ASTAlterCommand::MOVE_PARTITION; + command->part = true; + + if (s_to_disk.ignore(pos)) + command->move_destination_type = DataDestinationType::DISK; + else if (s_to_volume.ignore(pos)) + command->move_destination_type = DataDestinationType::VOLUME; + else if (s_to_table.ignore(pos)) + { + if (!parseDatabaseAndTableName(pos, expected, command->to_database, command->to_table)) + return false; + command->move_destination_type = DataDestinationType::TABLE; + } + else if (s_to_shard.ignore(pos)) + { + command->move_destination_type = DataDestinationType::SHARD; + } + else + return false; + + if (command->move_destination_type != DataDestinationType::TABLE) + { + ASTPtr ast_space_name; + if (!parser_string_literal.parse(pos, ast_space_name, expected)) + return false; + + command->move_destination_name = ast_space_name->as().value.get(); + } + } + else if (s_move_partition.ignore(pos, expected)) { if (!parser_partition.parse(pos, command->partition, expected)) return false; + + command->type = ASTAlterCommand::MOVE_PARTITION; + + if (s_to_disk.ignore(pos)) + command->move_destination_type = DataDestinationType::DISK; + else if (s_to_volume.ignore(pos)) + command->move_destination_type = DataDestinationType::VOLUME; + else if (s_to_table.ignore(pos)) + { + if (!parseDatabaseAndTableName(pos, expected, command->to_database, command->to_table)) + return false; + command->move_destination_type = DataDestinationType::TABLE; + } + else + return false; + + if (command->move_destination_type != DataDestinationType::TABLE) + { + ASTPtr ast_space_name; + if (!parser_string_literal.parse(pos, ast_space_name, expected)) + return false; + + command->move_destination_name = ast_space_name->as().value.get(); + } } - } - else if (s_modify_setting.ignore(pos, expected)) - { - if (!parser_settings.parse(pos, command->settings_changes, expected)) + else if (s_add_constraint.ignore(pos, expected)) + { + if (s_if_not_exists.ignore(pos, expected)) + command->if_not_exists = true; + + if (!parser_constraint_decl.parse(pos, command->constraint_decl, expected)) + return false; + + command->type = ASTAlterCommand::ADD_CONSTRAINT; + } + else if (s_drop_constraint.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->constraint, expected)) + return false; + + command->type = ASTAlterCommand::DROP_CONSTRAINT; + command->detach = false; + } + else if (s_detach_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + + command->type = ASTAlterCommand::DROP_PARTITION; + command->detach = true; + } + else if (s_detach_part.ignore(pos, expected)) + { + if (!parser_string_literal.parse(pos, command->partition, expected)) + return false; + + command->type = ASTAlterCommand::DROP_PARTITION; + command->part = true; + command->detach = true; + } + else if (s_attach_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + + if (s_from.ignore(pos)) + { + if (!parseDatabaseAndTableName(pos, expected, command->from_database, command->from_table)) + return false; + + command->replace = false; + command->type = ASTAlterCommand::REPLACE_PARTITION; + } + else + { + command->type = ASTAlterCommand::ATTACH_PARTITION; + } + } + else if (s_replace_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + + if (!s_from.ignore(pos, expected)) + return false; + + if (!parseDatabaseAndTableName(pos, expected, command->from_database, command->from_table)) + return false; + + command->replace = true; + command->type = ASTAlterCommand::REPLACE_PARTITION; + } + else if (s_attach_part.ignore(pos, expected)) + { + if (!parser_string_literal.parse(pos, command->partition, expected)) + return false; + + command->part = true; + command->type = ASTAlterCommand::ATTACH_PARTITION; + } + else if (s_fetch_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + + if (!s_from.ignore(pos, expected)) + return false; + + ASTPtr ast_from; + if (!parser_string_literal.parse(pos, ast_from, expected)) + return false; + + command->from = ast_from->as().value.get(); + command->type = ASTAlterCommand::FETCH_PARTITION; + } + else if (s_fetch_part.ignore(pos, expected)) + { + if (!parser_string_literal.parse(pos, command->partition, expected)) + return false; + + if (!s_from.ignore(pos, expected)) + return false; + + ASTPtr ast_from; + if (!parser_string_literal.parse(pos, ast_from, expected)) + return false; + command->from = ast_from->as().value.get(); + command->part = true; + command->type = ASTAlterCommand::FETCH_PARTITION; + } + else if (s_freeze.ignore(pos, expected)) + { + if (s_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + + command->type = ASTAlterCommand::FREEZE_PARTITION; + } + else + { + command->type = ASTAlterCommand::FREEZE_ALL; + } + + /// WITH NAME 'name' - place local backup to directory with specified name + if (s_with.ignore(pos, expected)) + { + if (!s_name.ignore(pos, expected)) + return false; + + ASTPtr ast_with_name; + if (!parser_string_literal.parse(pos, ast_with_name, expected)) + return false; + + command->with_name = ast_with_name->as().value.get(); + } + } + else if (s_unfreeze.ignore(pos, expected)) + { + if (s_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + + command->type = ASTAlterCommand::UNFREEZE_PARTITION; + } + else + { + command->type = ASTAlterCommand::UNFREEZE_ALL; + } + + /// WITH NAME 'name' - remove local backup to directory with specified name + if (s_with.ignore(pos, expected)) + { + if (!s_name.ignore(pos, expected)) + return false; + + ASTPtr ast_with_name; + if (!parser_string_literal.parse(pos, ast_with_name, expected)) + return false; + + command->with_name = ast_with_name->as().value.get(); + } + else + { + return false; + } + } + else if (s_modify_column.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_modify_col_decl.parse(pos, command->col_decl, expected)) + return false; + + if (s_remove.ignore(pos, expected)) + { + if (s_default.ignore(pos, expected)) + command->remove_property = "DEFAULT"; + else if (s_materialized.ignore(pos, expected)) + command->remove_property = "MATERIALIZED"; + else if (s_alias.ignore(pos, expected)) + command->remove_property = "ALIAS"; + else if (s_comment.ignore(pos, expected)) + command->remove_property = "COMMENT"; + else if (s_codec.ignore(pos, expected)) + command->remove_property = "CODEC"; + else if (s_ttl.ignore(pos, expected)) + command->remove_property = "TTL"; + else + return false; + } + else + { + if (s_first.ignore(pos, expected)) + command->first = true; + else if (s_after.ignore(pos, expected)) + { + if (!parser_name.parse(pos, command->column, expected)) + return false; + } + } + command->type = ASTAlterCommand::MODIFY_COLUMN; + } + else if (s_modify_order_by.ignore(pos, expected)) + { + if (!parser_exp_elem.parse(pos, command->order_by, expected)) + return false; + + command->type = ASTAlterCommand::MODIFY_ORDER_BY; + } + else if (s_modify_sample_by.ignore(pos, expected)) + { + if (!parser_exp_elem.parse(pos, command->sample_by, expected)) + return false; + + command->type = ASTAlterCommand::MODIFY_SAMPLE_BY; + } + else if (s_delete.ignore(pos, expected)) + { + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + + if (!s_where.ignore(pos, expected)) + return false; + + if (!parser_exp_elem.parse(pos, command->predicate, expected)) + return false; + + command->type = ASTAlterCommand::DELETE; + } + else if (s_update.ignore(pos, expected)) + { + if (!parser_assignment_list.parse(pos, command->update_assignments, expected)) + return false; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + + if (!s_where.ignore(pos, expected)) + return false; + + if (!parser_exp_elem.parse(pos, command->predicate, expected)) + return false; + + command->type = ASTAlterCommand::UPDATE; + } + else if (s_comment_column.ignore(pos, expected)) + { + if (s_if_exists.ignore(pos, expected)) + command->if_exists = true; + + if (!parser_name.parse(pos, command->column, expected)) + return false; + + if (!parser_string_literal.parse(pos, command->comment, expected)) + return false; + + command->type = ASTAlterCommand::COMMENT_COLUMN; + } + else if (s_modify_ttl.ignore(pos, expected)) + { + if (!parser_ttl_list.parse(pos, command->ttl, expected)) + return false; + command->type = ASTAlterCommand::MODIFY_TTL; + } + else if (s_remove_ttl.ignore(pos, expected)) + { + command->type = ASTAlterCommand::REMOVE_TTL; + } + else if (s_materialize_ttl.ignore(pos, expected)) + { + command->type = ASTAlterCommand::MATERIALIZE_TTL; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } + else if (s_modify_setting.ignore(pos, expected)) + { + if (!parser_settings.parse(pos, command->settings_changes, expected)) + return false; + command->type = ASTAlterCommand::MODIFY_SETTING; + } + else if (s_reset_setting.ignore(pos, expected)) + { + if (!parser_reset_setting.parse(pos, command->settings_resets, expected)) + return false; + command->type = ASTAlterCommand::RESET_SETTING; + } + else if (s_modify_query.ignore(pos, expected)) + { + if (!select_p.parse(pos, command->select, expected)) + return false; + command->type = ASTAlterCommand::MODIFY_QUERY; + } + else return false; - command->type = ASTAlterCommand::MODIFY_SETTING; } - else if (s_reset_setting.ignore(pos, expected)) - { - if (!parser_reset_setting.parse(pos, command->settings_resets, expected)) - return false; - command->type = ASTAlterCommand::RESET_SETTING; - } - else if (s_modify_query.ignore(pos, expected)) - { - if (!select_p.parse(pos, command->select, expected)) - return false; - command->type = ASTAlterCommand::MODIFY_QUERY; - } - else - return false; } if (command->col_decl) @@ -770,7 +786,7 @@ bool ParserAlterCommandList::parseImpl(Pos & pos, ASTPtr & node, Expected & expe node = command_list; ParserToken s_comma(TokenType::Comma); - ParserAlterCommand p_command(is_live_view); + ParserAlterCommand p_command(alter_object); do { @@ -793,19 +809,24 @@ bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) ParserKeyword s_alter_table("ALTER TABLE"); ParserKeyword s_alter_live_view("ALTER LIVE VIEW"); + ParserKeyword s_alter_database("ALTER DATABASE"); - bool is_live_view = false; + ASTAlterQuery::AlterObjectType alter_object_type; - if (!s_alter_table.ignore(pos, expected)) + if (s_alter_table.ignore(pos, expected)) { - if (!s_alter_live_view.ignore(pos, expected)) - return false; - else - is_live_view = true; + alter_object_type = ASTAlterQuery::AlterObjectType::TABLE; } - - if (is_live_view) - query->is_live_view = true; + else if (s_alter_live_view.ignore(pos, expected)) + { + alter_object_type = ASTAlterQuery::AlterObjectType::LIVE_VIEW; + } + else if (s_alter_database.ignore(pos, expected)) + { + alter_object_type = ASTAlterQuery::AlterObjectType::DATABASE; + } + else + return false; if (!parseDatabaseAndTableName(pos, expected, query->database, query->table)) return false; @@ -818,7 +839,7 @@ bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) } query->cluster = cluster_str; - ParserAlterCommandList p_command_list(is_live_view); + ParserAlterCommandList p_command_list(alter_object_type); ASTPtr command_list; if (!p_command_list.parse(pos, command_list, expected)) return false; diff --git a/src/Parsers/ParserAlterQuery.h b/src/Parsers/ParserAlterQuery.h index 2e54c4ddbaf..de9d752d1a3 100644 --- a/src/Parsers/ParserAlterQuery.h +++ b/src/Parsers/ParserAlterQuery.h @@ -2,6 +2,7 @@ #include #include +#include namespace DB { @@ -45,9 +46,10 @@ protected: bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; public: - bool is_live_view; + ASTAlterQuery::AlterObjectType alter_object; - ParserAlterCommandList(bool is_live_view_ = false) : is_live_view(is_live_view_) {} + ParserAlterCommandList(ASTAlterQuery::AlterObjectType alter_object_ = ASTAlterQuery::AlterObjectType::TABLE) + : alter_object(alter_object_) {} }; @@ -58,9 +60,10 @@ protected: bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; public: - bool is_live_view; + ASTAlterQuery::AlterObjectType alter_object; - ParserAlterCommand(bool is_live_view_ = false) : is_live_view(is_live_view_) {} + ParserAlterCommand(ASTAlterQuery::AlterObjectType alter_object_ = ASTAlterQuery::AlterObjectType::TABLE) + : alter_object(alter_object_) {} }; From 4cd62227cfa25044e61e22b1e631f46621492157 Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 27 Aug 2021 09:30:21 +0300 Subject: [PATCH 02/13] Almost done --- src/Access/AccessType.h | 2 + src/Common/ErrorCodes.cpp | 9 +- src/Core/PostgreSQL/Connection.cpp | 2 +- src/Core/PostgreSQL/Utils.cpp | 9 + src/Core/PostgreSQL/Utils.h | 4 + src/Databases/DatabaseAtomic.cpp | 66 ++++++- src/Databases/DatabaseAtomic.h | 7 +- src/Databases/DatabaseFactory.cpp | 23 ++- src/Databases/DatabaseReplicated.cpp | 5 +- src/Databases/DatabaseReplicated.h | 2 +- src/Databases/IDatabase.h | 17 ++ .../MySQL/DatabaseMaterializedMySQL.cpp | 8 +- .../MySQL/DatabaseMaterializedMySQL.h | 2 +- .../DatabaseMaterializedPostgreSQL.cpp | 133 ++++++++++++- .../DatabaseMaterializedPostgreSQL.h | 12 +- .../fetchPostgreSQLTableStructure.cpp | 8 + src/Interpreters/DatabaseCatalog.cpp | 2 +- src/Interpreters/InterpreterAlterQuery.cpp | 50 ++++- src/Interpreters/InterpreterAlterQuery.h | 5 + src/Parsers/ASTAlterQuery.cpp | 7 + src/Parsers/ASTAlterQuery.h | 7 +- src/Parsers/ParserAlterQuery.cpp | 31 +++- src/Parsers/parseDatabaseAndTableName.cpp | 16 ++ src/Parsers/parseDatabaseAndTableName.h | 2 + src/Storages/AlterCommands.cpp | 26 +++ src/Storages/AlterCommands.h | 5 + .../MaterializedPostgreSQLConsumer.cpp | 96 +++++++--- .../MaterializedPostgreSQLConsumer.h | 19 +- .../PostgreSQLReplicationHandler.cpp | 174 ++++++++++++++---- .../PostgreSQL/PostgreSQLReplicationHandler.h | 24 +-- .../StorageMaterializedPostgreSQL.cpp | 22 ++- .../StorageMaterializedPostgreSQL.h | 15 +- src/Storages/StorageInMemoryMetadata.h | 2 +- 33 files changed, 673 insertions(+), 139 deletions(-) diff --git a/src/Access/AccessType.h b/src/Access/AccessType.h index b1b49a6ba75..57342ee5503 100644 --- a/src/Access/AccessType.h +++ b/src/Access/AccessType.h @@ -71,6 +71,8 @@ enum class AccessType M(ALTER_FETCH_PARTITION, "ALTER FETCH PART, FETCH PARTITION", TABLE, ALTER_TABLE) \ M(ALTER_FREEZE_PARTITION, "FREEZE PARTITION, UNFREEZE", TABLE, ALTER_TABLE) \ \ + M(ALTER_DATABASE_SETTINGS, "ALTER DATABASE SETTING, ALTER MODIFY DATABASE SETTING, MODIFY DATABASE SETTING", TABLE, ALTER_TABLE) /* allows to execute ALTER MODIFY SETTING */\ + \ M(ALTER_TABLE, "", GROUP, ALTER) \ \ M(ALTER_VIEW_REFRESH, "ALTER LIVE VIEW REFRESH, REFRESH VIEW", VIEW, ALTER_VIEW) \ diff --git a/src/Common/ErrorCodes.cpp b/src/Common/ErrorCodes.cpp index 2110a8c7d6d..36d0fafdb4c 100644 --- a/src/Common/ErrorCodes.cpp +++ b/src/Common/ErrorCodes.cpp @@ -577,12 +577,9 @@ M(606, BACKUP_IS_EMPTY) \ M(607, BACKUP_ELEMENT_DUPLICATE) \ M(608, CANNOT_RESTORE_TABLE) \ - M(609, FUNCTION_ALREADY_EXISTS) \ - M(610, CANNOT_DROP_SYSTEM_FUNCTION) \ - M(611, CANNOT_CREATE_RECURSIVE_FUNCTION) \ - M(612, OBJECT_ALREADY_STORED_ON_DISK) \ - M(613, OBJECT_WAS_NOT_STORED_ON_DISK) \ - M(614, POSTGRESQL_CONNECTION_FAILURE) \ + M(609, POSTGRESQL_CONNECTION_FAILURE) \ + M(610, POSTGRESQL_REPLICATION_INTERNAL_ERROR) \ + M(611, QUERY_NOT_ALLOWED) \ \ M(999, KEEPER_EXCEPTION) \ M(1000, POCO_EXCEPTION) \ diff --git a/src/Core/PostgreSQL/Connection.cpp b/src/Core/PostgreSQL/Connection.cpp index e5c61c19963..2cb52fcff81 100644 --- a/src/Core/PostgreSQL/Connection.cpp +++ b/src/Core/PostgreSQL/Connection.cpp @@ -3,6 +3,7 @@ #if USE_LIBPQXX #include +#include namespace postgres { @@ -42,7 +43,6 @@ void Connection::execWithRetry(const std::function pqxx::connection & Connection::getRef() { connect(); - assert(connection != nullptr); return *connection; } diff --git a/src/Core/PostgreSQL/Utils.cpp b/src/Core/PostgreSQL/Utils.cpp index ebfdacd0fea..98cd706be69 100644 --- a/src/Core/PostgreSQL/Utils.cpp +++ b/src/Core/PostgreSQL/Utils.cpp @@ -19,6 +19,15 @@ ConnectionInfo formatConnectionString(String dbname, String host, UInt16 port, S return std::make_pair(out.str(), host + ':' + DB::toString(port)); } +String formatNameForLogs(const String & postgres_database_name, const String & postgres_table_name) +{ + if (postgres_database_name.empty()) + return postgres_table_name; + if (postgres_table_name.empty()) + return postgres_database_name; + return fmt::format("{}.{}", postgres_database_name, postgres_table_name); +} + } #endif diff --git a/src/Core/PostgreSQL/Utils.h b/src/Core/PostgreSQL/Utils.h index 4a58fcffb9a..59b44f8f5e1 100644 --- a/src/Core/PostgreSQL/Utils.h +++ b/src/Core/PostgreSQL/Utils.h @@ -19,7 +19,11 @@ namespace pqxx namespace postgres { + ConnectionInfo formatConnectionString(String dbname, String host, UInt16 port, String user, String password); + +String formatNameForLogs(const String & postgres_database_name, const String & postgres_table_name); + } #endif diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index b55277594be..8ce17198d3d 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -36,19 +37,20 @@ public: UUID uuid() const override { return table()->getStorageID().uuid; } }; -DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_) +DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_, ASTPtr storage_def_) : DatabaseOrdinary(name_, std::move(metadata_path_), "store/", logger_name, context_) , path_to_table_symlinks(fs::path(getContext()->getPath()) / "data" / escapeForFileName(name_) / "") , path_to_metadata_symlink(fs::path(getContext()->getPath()) / "metadata" / escapeForFileName(name_)) , db_uuid(uuid) + , storage_def(storage_def_) { assert(db_uuid != UUIDHelpers::Nil); fs::create_directories(path_to_table_symlinks); tryCreateMetadataSymlink(); } -DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_) - : DatabaseAtomic(name_, std::move(metadata_path_), uuid, "DatabaseAtomic (" + name_ + ")", context_) +DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_, ASTPtr storage_def_) + : DatabaseAtomic(name_, std::move(metadata_path_), uuid, "DatabaseAtomic (" + name_ + ")", context_, storage_def_) { } @@ -566,4 +568,62 @@ void DatabaseAtomic::checkDetachedTableNotInUse(const UUID & uuid) assertDetachedTableNotInUse(uuid); } +void DatabaseAtomic::modifySettings(const SettingsChanges & settings_changes, ContextPtr local_context) +{ + applySettings(settings_changes, local_context); + + ASTCreateQuery create; + create.attach = true; + create.database = "_"; + create.uuid = getUUID(); + create.if_not_exists = false; + create.storage = assert_cast(storage_def.get()); + auto * ast_set_query = create.storage->settings; + + if (ast_set_query) + { + auto & previous_settings = ast_set_query->changes; + for (const auto & change : settings_changes) + { + auto it = std::find_if(previous_settings.begin(), previous_settings.end(), + [&](const auto & prev){ return prev.name == change.name; }); + if (it != previous_settings.end()) + it->value = change.value; + else + previous_settings.push_back(change); + } + } + else + { + auto settings = std::make_shared(); + settings->is_standalone = false; + settings->changes = settings_changes; + create.storage->set(create.storage->settings, settings->clone()); + } + + create.attach = true; + create.if_not_exists = false; + + WriteBufferFromOwnString statement_buf; + formatAST(create, statement_buf, false); + writeChar('\n', statement_buf); + String statement = statement_buf.str(); + + String database_name_escaped = escapeForFileName(database_name); + fs::path metadata_root_path = fs::canonical(local_context->getGlobalContext()->getPath()); + fs::path metadata_file_tmp_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql.tmp"); + fs::path metadata_file_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql"); + + /// Exclusive flag guarantees, that database is not created right now in another thread. + WriteBufferFromFile out(metadata_file_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); + writeString(statement, out); + + out.next(); + if (getContext()->getSettingsRef().fsync_metadata) + out.sync(); + out.close(); + + fs::rename(metadata_file_tmp_path, metadata_file_path); +} + } diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index 21e841841bd..0b00a4eb43a 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -19,8 +19,8 @@ namespace DB class DatabaseAtomic : public DatabaseOrdinary { public: - DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_); - DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_); + DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_, ASTPtr storage_def_); + DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_, ASTPtr storage_def_); String getEngineName() const override { return "Atomic"; } UUID getUUID() const override { return db_uuid; } @@ -61,6 +61,8 @@ public: void checkDetachedTableNotInUse(const UUID & uuid) override; void setDetachedTableNotInUseForce(const UUID & uuid); + void modifySettings(const SettingsChanges & settings_changes, ContextPtr local_context) override; + protected: void commitAlterTable(const StorageID & table_id, const String & table_metadata_tmp_path, const String & table_metadata_path, const String & statement, ContextPtr query_context) override; void commitCreateTable(const ASTCreateQuery & query, const StoragePtr & table, @@ -80,6 +82,7 @@ protected: String path_to_table_symlinks; String path_to_metadata_symlink; const UUID db_uuid; + ASTPtr storage_def; }; } diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 75a3b9c9e1e..cff71a0e7fd 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -103,13 +103,20 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String const String & engine_name = engine_define->engine->name; const UUID & uuid = create.uuid; + static const std::unordered_set database_engines{"Ordinary", "Atomic", "Memory", + "Dictionary", "Lazy", "Replicated", "MySQL", "MaterializeMySQL", "MaterializedMySQL", + "Lazy", "Replicated", "PostgreSQL", "MaterializedPostgreSQL", "SQLite"}; + + if (!database_engines.contains(engine_name)) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine name `{}` does not exist", engine_name); + static const std::unordered_set engines_with_arguments{"MySQL", "MaterializeMySQL", "MaterializedMySQL", "Lazy", "Replicated", "PostgreSQL", "MaterializedPostgreSQL", "SQLite"}; bool engine_may_have_arguments = engines_with_arguments.contains(engine_name); if (engine_define->engine->arguments && !engine_may_have_arguments) - throw Exception("Database engine " + engine_name + " cannot have arguments", ErrorCodes::BAD_ARGUMENTS); + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine `{}` cannot have arguments", engine_name); bool has_unexpected_element = engine_define->engine->parameters || engine_define->partition_by || engine_define->primary_key || engine_define->order_by || @@ -117,13 +124,13 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String bool may_have_settings = endsWith(engine_name, "MySQL") || engine_name == "Replicated" || engine_name == "MaterializedPostgreSQL"; if (has_unexpected_element || (!may_have_settings && engine_define->settings)) - throw Exception("Database engine " + engine_name + " cannot have parameters, primary_key, order_by, sample_by, settings", - ErrorCodes::UNKNOWN_ELEMENT_IN_AST); + throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_AST, + "Database engine `{}` cannot have parameters, primary_key, order_by, sample_by, settings", engine_name); if (engine_name == "Ordinary") return std::make_shared(database_name, metadata_path, context); else if (engine_name == "Atomic") - return std::make_shared(database_name, metadata_path, uuid, context); + return std::make_shared(database_name, metadata_path, uuid, context, engine_define->clone()); else if (engine_name == "Memory") return std::make_shared(database_name, context); else if (engine_name == "Dictionary") @@ -177,11 +184,11 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String if (create.uuid == UUIDHelpers::Nil) return std::make_shared>( context, database_name, metadata_path, uuid, mysql_database_name, std::move(mysql_pool), std::move(client) - , std::move(materialize_mode_settings)); + , std::move(materialize_mode_settings), engine_define->clone()); else return std::make_shared>( context, database_name, metadata_path, uuid, mysql_database_name, std::move(mysql_pool), std::move(client) - , std::move(materialize_mode_settings)); + , std::move(materialize_mode_settings), engine_define->clone()); } catch (...) { @@ -227,7 +234,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String return std::make_shared(database_name, metadata_path, uuid, zookeeper_path, shard_name, replica_name, - std::move(database_replicated_settings), context); + std::move(database_replicated_settings), context, engine_define->clone()); } #if USE_LIBPQXX @@ -304,7 +311,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String postgresql_replica_settings->loadFromQuery(*engine_define); return std::make_shared( - context, metadata_path, uuid, engine_define, create.attach, + context, metadata_path, uuid, engine_define->clone(), create.attach, database_name, postgres_database_name, connection_info, std::move(postgresql_replica_settings)); } diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 8e8fb4e2d6d..d8835160151 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -67,8 +67,9 @@ DatabaseReplicated::DatabaseReplicated( const String & shard_name_, const String & replica_name_, DatabaseReplicatedSettings db_settings_, - ContextPtr context_) - : DatabaseAtomic(name_, metadata_path_, uuid, "DatabaseReplicated (" + name_ + ")", context_) + ContextPtr context_, + ASTPtr storage_def_) + : DatabaseAtomic(name_, metadata_path_, uuid, "DatabaseReplicated (" + name_ + ")", context_, storage_def_) , zookeeper_path(zookeeper_path_) , shard_name(shard_name_) , replica_name(replica_name_) diff --git a/src/Databases/DatabaseReplicated.h b/src/Databases/DatabaseReplicated.h index 41b1bf13e5f..997262325f6 100644 --- a/src/Databases/DatabaseReplicated.h +++ b/src/Databases/DatabaseReplicated.h @@ -24,7 +24,7 @@ public: DatabaseReplicated(const String & name_, const String & metadata_path_, UUID uuid, const String & zookeeper_path_, const String & shard_name_, const String & replica_name_, DatabaseReplicatedSettings db_settings_, - ContextPtr context); + ContextPtr context, ASTPtr storage_def_); ~DatabaseReplicated() override; diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index 387c6882eab..f3f801e620b 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -24,6 +24,8 @@ struct IndicesDescription; struct StorageInMemoryMetadata; struct StorageID; class ASTCreateQuery; +class AlterCommands; +class SettingsChanges; using DictionariesWithID = std::vector>; namespace ErrorCodes @@ -272,6 +274,21 @@ public: /// Delete data and metadata stored inside the database, if exists. virtual void drop(ContextPtr /*context*/) {} + virtual void checkAlterIsPossible(const AlterCommands & /* commands */, ContextPtr /* context */) const + { + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter is not supported by database engine {}", getEngineName()); + } + + virtual void modifySettings(const SettingsChanges &, ContextPtr) + { + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} does not support settings", getEngineName()); + } + + virtual void applySettings(const SettingsChanges &, ContextPtr) + { + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} does not support settings", getEngineName()); + } + virtual ~IDatabase() = default; protected: diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index ba9b30425dd..644e89894c4 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -37,7 +37,8 @@ DatabaseMaterializedMySQL::DatabaseMaterializedMySQL( const String & mysql_database_name_, mysqlxx::Pool && pool_, MySQLClient && client_, - std::unique_ptr settings_) + std::unique_ptr settings_, + ASTPtr) : DatabaseOrdinary( database_name_, metadata_path_, @@ -58,8 +59,9 @@ DatabaseMaterializedMySQL::DatabaseMaterializedMySQL( const String & mysql_database_name_, mysqlxx::Pool && pool_, MySQLClient && client_, - std::unique_ptr settings_) - : DatabaseAtomic(database_name_, metadata_path_, uuid, "DatabaseMaterializedMySQL (" + database_name_ + ")", context_) + std::unique_ptr settings_, + ASTPtr storage_def_) + : DatabaseAtomic(database_name_, metadata_path_, uuid, "DatabaseMaterializedMySQL (" + database_name_ + ")", context_, storage_def_) , settings(std::move(settings_)) , materialize_thread(context_, database_name_, mysql_database_name_, std::move(pool_), std::move(client_), settings.get()) { diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.h b/src/Databases/MySQL/DatabaseMaterializedMySQL.h index 812a0fb64c8..7bfa310b4cf 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.h +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.h @@ -25,7 +25,7 @@ public: DatabaseMaterializedMySQL( ContextPtr context, const String & database_name_, const String & metadata_path_, UUID uuid, const String & mysql_database_name_, mysqlxx::Pool && pool_, - MySQLClient && client_, std::unique_ptr settings_); + MySQLClient && client_, std::unique_ptr settings_, ASTPtr storage_def_); void rethrowExceptionIfNeed() const; diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index fdd181373df..b4b0037ff1b 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -10,12 +10,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -30,20 +32,20 @@ namespace ErrorCodes { extern const int NOT_IMPLEMENTED; extern const int LOGICAL_ERROR; + extern const int QUERY_NOT_ALLOWED; } DatabaseMaterializedPostgreSQL::DatabaseMaterializedPostgreSQL( ContextPtr context_, const String & metadata_path_, UUID uuid_, - const ASTStorage * database_engine_define_, + ASTPtr storage_def_, bool is_attach_, const String & database_name_, const String & postgres_database_name, const postgres::ConnectionInfo & connection_info_, std::unique_ptr settings_) - : DatabaseAtomic(database_name_, metadata_path_, uuid_, "DatabaseMaterializedPostgreSQL (" + database_name_ + ")", context_) - , database_engine_define(database_engine_define_->clone()) + : DatabaseAtomic(database_name_, metadata_path_, uuid_, "DatabaseMaterializedPostgreSQL (" + database_name_ + ")", context_, storage_def_) , is_attach(is_attach_) , remote_database_name(postgres_database_name) , connection_info(connection_info_) @@ -66,11 +68,10 @@ void DatabaseMaterializedPostgreSQL::startSynchronization() /* is_materialized_postgresql_database = */ true, settings->materialized_postgresql_tables_list.value); - postgres::Connection connection(connection_info); NameSet tables_to_replicate; try { - tables_to_replicate = replication_handler->fetchRequiredTables(connection); + tables_to_replicate = replication_handler->fetchRequiredTables(); } catch (...) { @@ -89,12 +90,12 @@ void DatabaseMaterializedPostgreSQL::startSynchronization() if (storage) { /// Nested table was already created and synchronized. - storage = StorageMaterializedPostgreSQL::create(storage, getContext()); + storage = StorageMaterializedPostgreSQL::create(storage, getContext(), remote_database_name, table_name); } else { /// Nested table does not exist and will be created by replication thread. - storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext()); + storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); } /// Cache MaterializedPostgreSQL wrapper over nested table. @@ -124,7 +125,34 @@ void DatabaseMaterializedPostgreSQL::loadStoredObjects(ContextMutablePtr local_c if (!force_attach) throw; } +} + +void DatabaseMaterializedPostgreSQL::checkAlterIsPossible(const AlterCommands & commands, ContextPtr) const +{ + for (const auto & command : commands) + { + if (command.type != AlterCommand::MODIFY_DATABASE_SETTING) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter of type '{}' is not supported by database engine {}", alterTypeToString(command.type), getEngineName()); + } +} + + +void DatabaseMaterializedPostgreSQL::applySettings(const SettingsChanges & settings_changes, ContextPtr local_context) +{ + for (const auto & change : settings_changes) + { + if (!settings->has(change.name)) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine {} does not support setting `{}`", getEngineName(), change.name); + + if (change.name == "materialized_postgresql_tables_list") + { + if (local_context->isInternalQuery() || materialized_tables.empty()) + throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "Changin settings `{}` is allowed only internally. Use CREATE TABLE query", change.name); + } + + settings->applyChange(change); + } } @@ -164,8 +192,38 @@ void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const return; } - throw Exception(ErrorCodes::NOT_IMPLEMENTED, - "Create table query allowed only for ReplacingMergeTree engine and from synchronization thread"); + throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "CREATE TABLE is not allowed for database engine {}", getEngineName()); +} + + +void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, const StoragePtr & table, const String & relative_table_path) +{ + if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) + { + auto set = std::make_shared(); + set->is_standalone = false; + auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; + set->changes = {SettingChange("materialized_postgresql_tables_list", tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name))}; + + auto command = std::make_shared(); + command->type = ASTAlterCommand::Type::MODIFY_DATABASE_SETTING; + command->children.emplace_back(std::move(set)); + + auto expr = std::make_shared(); + expr->children.push_back(command); + + ASTAlterQuery alter; + alter.alter_object = ASTAlterQuery::AlterObjectType::DATABASE; + alter.children.emplace_back(std::move(expr)); + + auto storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); + materialized_tables[table_name] = storage; + replication_handler->addTableToReplication(dynamic_cast(storage.get()), table_name); + } + else + { + DatabaseAtomic::attachTable(table_name, table, relative_table_path); + } } @@ -209,6 +267,63 @@ DatabaseTablesIteratorPtr DatabaseMaterializedPostgreSQL::getTablesIterator( return DatabaseAtomic::getTablesIterator(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), filter_by_table_name); } +static ASTPtr getColumnDeclaration(const DataTypePtr & data_type) +{ + WhichDataType which(data_type); + + if (which.isNullable()) + return makeASTFunction("Nullable", getColumnDeclaration(typeid_cast(data_type.get())->getNestedType())); + + if (which.isArray()) + return makeASTFunction("Array", getColumnDeclaration(typeid_cast(data_type.get())->getNestedType())); + + return std::make_shared(data_type->getName()); +} + + +ASTPtr DatabaseMaterializedPostgreSQL::getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const +{ + if (!local_context->hasQueryContext()) + return DatabaseAtomic::getCreateTableQueryImpl(table_name, local_context, throw_on_error); + + /// Note: here we make an assumption that table structure will not change between call to this method and to attachTable(). + auto storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); + replication_handler->addStructureToMaterializedStorage(storage.get(), table_name); + + auto create_table_query = std::make_shared(); + auto table_storage_define = storage_def->clone(); + create_table_query->set(create_table_query->storage, table_storage_define); + + auto columns_declare_list = std::make_shared(); + auto columns_expression_list = std::make_shared(); + + columns_declare_list->set(columns_declare_list->columns, columns_expression_list); + create_table_query->set(create_table_query->columns_list, columns_declare_list); + + /// init create query. + auto table_id = storage->getStorageID(); + create_table_query->table = table_id.table_name; + create_table_query->database = table_id.database_name; + + auto metadata_snapshot = storage->getInMemoryMetadataPtr(); + for (const auto & column_type_and_name : metadata_snapshot->getColumns().getAllPhysical()) + { + const auto & column_declaration = std::make_shared(); + column_declaration->name = column_type_and_name.name; + column_declaration->type = getColumnDeclaration(column_type_and_name.type); + columns_expression_list->children.emplace_back(column_declaration); + } + + ASTStorage * ast_storage = table_storage_define->as(); + ASTs storage_children = ast_storage->children; + auto storage_engine_arguments = ast_storage->engine->arguments; + /// Add table_name to engine arguments + storage_engine_arguments->children.insert(storage_engine_arguments->children.begin() + 2, std::make_shared(table_id.table_name)); + + return create_table_query; +} + + } diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index dd8b4dc438a..3c75af6cc9e 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -32,7 +32,7 @@ public: ContextPtr context_, const String & metadata_path_, UUID uuid_, - const ASTStorage * database_engine_define_, + ASTPtr storage_def_, bool is_attach_, const String & database_name_, const String & postgres_database_name, @@ -52,18 +52,26 @@ public: void createTable(ContextPtr context, const String & name, const StoragePtr & table, const ASTPtr & query) override; + void attachTable(const String & name, const StoragePtr & table, const String & relative_table_path) override; + void dropTable(ContextPtr local_context, const String & name, bool no_delay) override; void drop(ContextPtr local_context) override; void stopReplication(); + void checkAlterIsPossible(const AlterCommands & commands, ContextPtr context) const override; + + void applySettings(const SettingsChanges & settings_changes, ContextPtr context) override; + void shutdown() override; +protected: + ASTPtr getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const override; + private: void startSynchronization(); - ASTPtr database_engine_define; bool is_attach; String remote_database_name; postgres::ConnectionInfo connection_info; diff --git a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp index 1b77947264e..0495dd8723a 100644 --- a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp +++ b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp @@ -290,12 +290,20 @@ PostgreSQLTableStructure fetchPostgreSQLTableStructure( pqxx::ReplicationTransaction & tx, const String & postgres_table_name, bool use_nulls, bool with_primary_key, bool with_replica_identity_index); +template +PostgreSQLTableStructure fetchPostgreSQLTableStructure( + pqxx::nontransaction & tx, const String & postgres_table_name, bool use_nulls, + bool with_primary_key, bool with_replica_identity_index); + template std::unordered_set fetchPostgreSQLTablesList(pqxx::work & tx, const String & postgres_schema); template std::unordered_set fetchPostgreSQLTablesList(pqxx::ReadTransaction & tx, const String & postgres_schema); +template +std::unordered_set fetchPostgreSQLTablesList(pqxx::nontransaction & tx, const String & postgres_schema); + } #endif diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index fd6b5b9a810..ee49333a332 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -241,7 +241,7 @@ DatabaseAndTable DatabaseCatalog::getTableImpl( #if USE_LIBPQXX if (!context_->isInternalQuery() && (db_and_table.first->getEngineName() == "MaterializedPostgreSQL")) { - db_and_table.second = std::make_shared(std::move(db_and_table.second), getContext()); + db_and_table.second = std::make_shared(std::move(db_and_table.second), getContext(), "", db_and_table.second->getStorageID().table_name); } #endif diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index 76e7afb7009..2b4c5f3a8fa 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -39,12 +39,25 @@ InterpreterAlterQuery::InterpreterAlterQuery(const ASTPtr & query_ptr_, ContextP { } + BlockIO InterpreterAlterQuery::execute() { - BlockIO res; const auto & alter = query_ptr->as(); + std::cerr << "\n\n\n" << query_ptr->dumpTree() << std::endl; + if (alter.alter_object == ASTAlterQuery::AlterObjectType::DATABASE) + return executeToDatabase(alter); + else if (alter.alter_object == ASTAlterQuery::AlterObjectType::DATABASE) + return executeToTable(alter); + else if (alter.alter_object == ASTAlterQuery::AlterObjectType::LIVE_VIEW) + return executeToTable(alter); + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown alter"); +} +BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter) +{ + BlockIO res; + if (!alter.cluster.empty()) return executeDDLQueryOnCluster(query_ptr, getContext(), getRequiredAccess()); @@ -78,7 +91,9 @@ BlockIO InterpreterAlterQuery::execute() { auto * command_ast = child->as(); if (auto alter_command = AlterCommand::parse(command_ast)) + { alter_commands.emplace_back(std::move(*alter_command)); + } else if (auto partition_command = PartitionCommand::parse(command_ast)) { partition_commands.emplace_back(std::move(*partition_command)); @@ -92,7 +107,9 @@ BlockIO InterpreterAlterQuery::execute() mutation_commands.emplace_back(std::move(*mut_command)); } else if (auto live_view_command = LiveViewCommand::parse(command_ast)) + { live_view_commands.emplace_back(std::move(*live_view_command)); + } else throw Exception("Wrong parameter type in ALTER query", ErrorCodes::LOGICAL_ERROR); } @@ -149,6 +166,30 @@ BlockIO InterpreterAlterQuery::execute() } +BlockIO InterpreterAlterQuery::executeToDatabase(const ASTAlterQuery & alter) +{ + BlockIO res; + getContext()->checkAccess(getRequiredAccess()); + DatabasePtr database = DatabaseCatalog::instance().getDatabase(alter.database); + AlterCommands alter_commands; + + for (const auto & child : alter.command_list->children) + { + auto * command_ast = child->as(); + if (auto alter_command = AlterCommand::parse(command_ast)) + alter_commands.emplace_back(std::move(*alter_command)); + else + throw Exception("Wrong parameter type in ALTER DATABASE query", ErrorCodes::LOGICAL_ERROR); + } + + if (!alter_commands.empty()) + { + database->checkAlterIsPossible(alter_commands, getContext()); + alter_commands.apply(database, getContext()); + } + + return res; +} AccessRightsElements InterpreterAlterQuery::getRequiredAccess() const { AccessRightsElements required_access; @@ -343,6 +384,11 @@ AccessRightsElements InterpreterAlterQuery::getRequiredAccessForCommand(const AS required_access.emplace_back(AccessType::ALTER_RENAME_COLUMN, database, table, column_name()); break; } + case ASTAlterCommand::MODIFY_DATABASE_SETTING: + { + required_access.emplace_back(AccessType::ALTER_DATABASE_SETTINGS, database, table); + break; + } case ASTAlterCommand::NO_TYPE: break; } @@ -354,7 +400,7 @@ void InterpreterAlterQuery::extendQueryLogElemImpl(QueryLogElement & elem, const const auto & alter = ast->as(); elem.query_kind = "Alter"; - if (alter.command_list != nullptr) + if (alter.command_list != nullptr && alter.alter_object != ASTAlterQuery::AlterObjectType::DATABASE) { // Alter queries already have their target table inserted into `elem`. if (elem.query_tables.size() != 1) diff --git a/src/Interpreters/InterpreterAlterQuery.h b/src/Interpreters/InterpreterAlterQuery.h index ae9750b0b62..9494a400e7b 100644 --- a/src/Interpreters/InterpreterAlterQuery.h +++ b/src/Interpreters/InterpreterAlterQuery.h @@ -9,6 +9,7 @@ namespace DB class AccessRightsElements; class ASTAlterCommand; +class ASTAlterQuery; /** Allows you add or remove a column in the table. @@ -28,6 +29,10 @@ public: private: AccessRightsElements getRequiredAccess() const; + BlockIO executeToTable(const ASTAlterQuery & alter); + + BlockIO executeToDatabase(const ASTAlterQuery & alter); + ASTPtr query_ptr; }; diff --git a/src/Parsers/ASTAlterQuery.cpp b/src/Parsers/ASTAlterQuery.cpp index 0e617ca7c21..d0adb2e799f 100644 --- a/src/Parsers/ASTAlterQuery.cpp +++ b/src/Parsers/ASTAlterQuery.cpp @@ -389,6 +389,11 @@ void ASTAlterCommand::formatImpl( settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << "RESET SETTING " << (settings.hilite ? hilite_none : ""); settings_resets->formatImpl(settings, state, frame); } + else if (type == ASTAlterCommand::MODIFY_DATABASE_SETTING) + { + settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << "MODIFY SETTING " << (settings.hilite ? hilite_none : ""); + settings_changes->formatImpl(settings, state, frame); + } else if (type == ASTAlterCommand::MODIFY_QUERY) { settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << "MODIFY QUERY " << settings.nl_or_ws << (settings.hilite ? hilite_none : ""); @@ -474,6 +479,8 @@ void ASTAlterQuery::formatQueryImpl(const FormatSettings & settings, FormatState case AlterObjectType::LIVE_VIEW: settings.ostr << "ALTER LIVE VIEW "; break; + default: + break; } settings.ostr << (settings.hilite ? hilite_none : ""); diff --git a/src/Parsers/ASTAlterQuery.h b/src/Parsers/ASTAlterQuery.h index dadba107ddc..d99bac80fed 100644 --- a/src/Parsers/ASTAlterQuery.h +++ b/src/Parsers/ASTAlterQuery.h @@ -68,6 +68,8 @@ public: NO_TYPE, LIVE_VIEW_REFRESH, + + MODIFY_DATABASE_SETTING, }; Type type = NO_TYPE; @@ -212,11 +214,12 @@ public: { TABLE, DATABASE, - LIVE_VIEW + LIVE_VIEW, + UNKNOWN, }; // bool is_live_view{false}; /// true for ALTER LIVE VIEW - AlterObjectType alter_object = AlterObjectType::TABLE; + AlterObjectType alter_object = AlterObjectType::UNKNOWN; ASTExpressionList * command_list = nullptr; diff --git a/src/Parsers/ParserAlterQuery.cpp b/src/Parsers/ParserAlterQuery.cpp index cb1796a70b5..e8e0db8c492 100644 --- a/src/Parsers/ParserAlterQuery.cpp +++ b/src/Parsers/ParserAlterQuery.cpp @@ -141,12 +141,14 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected { if (!parser_settings.parse(pos, command->settings_changes, expected)) return false; - command->type = ASTAlterCommand::MODIFY_SETTING; + command->type = ASTAlterCommand::MODIFY_DATABASE_SETTING; } else return false; break; } + default: + break; case ASTAlterQuery::AlterObjectType::TABLE: { if (s_add_column.ignore(pos, expected)) @@ -828,16 +830,27 @@ bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) else return false; - if (!parseDatabaseAndTableName(pos, expected, query->database, query->table)) - return false; - - String cluster_str; - if (ParserKeyword{"ON"}.ignore(pos, expected)) + if (alter_object_type == ASTAlterQuery::AlterObjectType::DATABASE) { - if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected)) + std::cerr << "\n\n\nOK!\n\n"; + if (!parseDatabase(pos, expected, query->database)) return false; + std::cerr << "database name: " << query->database << std::endl; + } + else + { + std::cerr << "\n\n\nNOT OK!\n\n"; + if (!parseDatabaseAndTableName(pos, expected, query->database, query->table)) + return false; + + String cluster_str; + if (ParserKeyword{"ON"}.ignore(pos, expected)) + { + if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected)) + return false; + } + query->cluster = cluster_str; } - query->cluster = cluster_str; ParserAlterCommandList p_command_list(alter_object_type); ASTPtr command_list; @@ -845,6 +858,8 @@ bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) return false; query->set(query->command_list, command_list); + query->alter_object = alter_object_type; + std::cerr << "\n\n\nalter query: " << query->dumpTree() << std::endl; return true; } diff --git a/src/Parsers/parseDatabaseAndTableName.cpp b/src/Parsers/parseDatabaseAndTableName.cpp index 13429df5b4d..c071f1b6eb4 100644 --- a/src/Parsers/parseDatabaseAndTableName.cpp +++ b/src/Parsers/parseDatabaseAndTableName.cpp @@ -42,6 +42,22 @@ bool parseDatabaseAndTableName(IParser::Pos & pos, Expected & expected, String & } +bool parseDatabase(IParser::Pos & pos, Expected & expected, String & database_str) +{ + ParserToken s_dot(TokenType::Dot); + ParserIdentifier identifier_parser; + + ASTPtr database; + database_str = ""; + + if (!identifier_parser.parse(pos, database, expected)) + return false; + + tryGetIdentifierNameInto(database, database_str); + return true; +} + + bool parseDatabaseAndTableNameOrAsterisks(IParser::Pos & pos, Expected & expected, String & database, bool & any_database, String & table, bool & any_table) { return IParserBase::wrapParseImpl(pos, [&] diff --git a/src/Parsers/parseDatabaseAndTableName.h b/src/Parsers/parseDatabaseAndTableName.h index e4699c8ad91..dc435ca047e 100644 --- a/src/Parsers/parseDatabaseAndTableName.h +++ b/src/Parsers/parseDatabaseAndTableName.h @@ -10,4 +10,6 @@ bool parseDatabaseAndTableName(IParser::Pos & pos, Expected & expected, String & /// Parses [db.]name or [db.]* or [*.]* bool parseDatabaseAndTableNameOrAsterisks(IParser::Pos & pos, Expected & expected, String & database, bool & any_database, String & table, bool & any_table); +bool parseDatabase(IParser::Pos & pos, Expected & expected, String & database_str); + } diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 7388f44fee1..de9d9e594fd 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -312,6 +312,14 @@ std::optional AlterCommand::parse(const ASTAlterCommand * command_ command.settings_changes = command_ast->settings_changes->as().changes; return command; } + else if (command_ast->type == ASTAlterCommand::MODIFY_DATABASE_SETTING) + { + AlterCommand command; + command.ast = command_ast->clone(); + command.type = AlterCommand::MODIFY_DATABASE_SETTING; + command.settings_changes = command_ast->settings_changes->as().changes; + return command; + } else if (command_ast->type == ASTAlterCommand::RESET_SETTING) { AlterCommand command; @@ -350,6 +358,21 @@ std::optional AlterCommand::parse(const ASTAlterCommand * command_ } +void AlterCommands::apply(DatabasePtr database, ContextPtr context) const +{ + for (const AlterCommand & command : *this) + { + if (!command.ignore) + { + if (command.type == AlterCommand::MODIFY_DATABASE_SETTING) + database->modifySettings(command.settings_changes, context); + else + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unsupported alter command"); + } + } +} + + void AlterCommand::apply(StorageInMemoryMetadata & metadata, ContextPtr context) const { if (type == ADD_COLUMN) @@ -877,6 +900,8 @@ String alterTypeToString(const AlterCommand::Type type) return "MODIFY SETTING"; case AlterCommand::Type::RESET_SETTING: return "RESET SETTING"; + case AlterCommand::Type::MODIFY_DATABASE_SETTING: + return "MODIFY DATABASE SETTING"; case AlterCommand::Type::MODIFY_QUERY: return "MODIFY QUERY"; case AlterCommand::Type::RENAME_COLUMN: @@ -1007,6 +1032,7 @@ void AlterCommands::prepare(const StorageInMemoryMetadata & metadata) prepared = true; } + void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPtr context) const { auto all_columns = metadata.columns; diff --git a/src/Storages/AlterCommands.h b/src/Storages/AlterCommands.h index 60f4ad7d552..a2a1a3b6709 100644 --- a/src/Storages/AlterCommands.h +++ b/src/Storages/AlterCommands.h @@ -13,6 +13,8 @@ namespace DB { class ASTAlterCommand; +class IDatabase; +using DatabasePtr = std::shared_ptr; /// Operation from the ALTER query (except for manipulation with PART/PARTITION). /// Adding Nested columns is not expanded to add individual columns. @@ -42,6 +44,7 @@ struct AlterCommand MODIFY_QUERY, RENAME_COLUMN, REMOVE_TTL, + MODIFY_DATABASE_SETTING, }; /// Which property user wants to remove from column @@ -194,6 +197,8 @@ public: /// Commands have to be prepared before apply. void apply(StorageInMemoryMetadata & metadata, ContextPtr context) const; + void apply(DatabasePtr database, ContextPtr context) const; + /// At least one command modify settings. bool hasSettingsAlterCommand() const; diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp index b43e7656084..f9c448e9a5d 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp @@ -26,8 +26,9 @@ MaterializedPostgreSQLConsumer::MaterializedPostgreSQLConsumer( const std::string & start_lsn, const size_t max_block_size_, bool allow_automatic_update_, - Storages storages_) - : log(&Poco::Logger::get("PostgreSQLReaplicaConsumer")) + Storages storages_, + const String & name_for_logger) + : log(&Poco::Logger::get("PostgreSQLReplicaConsumer("+ name_for_logger +")")) , context(context_) , replication_slot_name(replication_slot_name_) , publication_name(publication_name_) @@ -270,12 +271,13 @@ void MaterializedPostgreSQLConsumer::processReplicationMessage(const char * repl case 'I': // Insert { Int32 relation_id = readInt32(replication_message, pos, size); + const auto & table_name = relation_id_to_name[relation_id]; + assert(!table_name.empty()); - if (!isSyncAllowed(relation_id)) + if (!isSyncAllowed(relation_id, table_name)) return; Int8 new_tuple = readInt8(replication_message, pos, size); - const auto & table_name = relation_id_to_name[relation_id]; auto buffer = buffers.find(table_name); assert(buffer != buffers.end()); @@ -287,11 +289,12 @@ void MaterializedPostgreSQLConsumer::processReplicationMessage(const char * repl case 'U': // Update { Int32 relation_id = readInt32(replication_message, pos, size); + const auto & table_name = relation_id_to_name[relation_id]; + assert(!table_name.empty()); - if (!isSyncAllowed(relation_id)) + if (!isSyncAllowed(relation_id, table_name)) return; - const auto & table_name = relation_id_to_name[relation_id]; auto buffer = buffers.find(table_name); assert(buffer != buffers.end()); @@ -335,14 +338,15 @@ void MaterializedPostgreSQLConsumer::processReplicationMessage(const char * repl case 'D': // Delete { Int32 relation_id = readInt32(replication_message, pos, size); + const auto & table_name = relation_id_to_name[relation_id]; + assert(!table_name.empty()); - if (!isSyncAllowed(relation_id)) + if (!isSyncAllowed(relation_id, table_name)) return; /// 0 or 1 if replica identity is set to full. For now only default replica identity is supported (with primary keys). readInt8(replication_message, pos, size); - const auto & table_name = relation_id_to_name[relation_id]; auto buffer = buffers.find(table_name); assert(buffer != buffers.end()); readTupleData(buffer->second, replication_message, pos, size, PostgreSQLQuery::DELETE); @@ -357,7 +361,7 @@ void MaterializedPostgreSQLConsumer::processReplicationMessage(const char * repl constexpr size_t transaction_commit_timestamp_len = 8; pos += unused_flags_len + commit_lsn_len + transaction_end_lsn_len + transaction_commit_timestamp_len; - LOG_DEBUG(log, "Current lsn: {} = {}", current_lsn, getLSNValue(current_lsn)); /// Will be removed + // LOG_DEBUG(log, "Current lsn: {} = {}", current_lsn, getLSNValue(current_lsn)); /// Will be removed final_lsn = current_lsn; break; @@ -371,7 +375,7 @@ void MaterializedPostgreSQLConsumer::processReplicationMessage(const char * repl readString(replication_message, pos, size, relation_namespace); readString(replication_message, pos, size, relation_name); - if (!isSyncAllowed(relation_id)) + if (!isSyncAllowed(relation_id, relation_name)) return; if (storages.find(relation_name) == storages.end()) @@ -522,15 +526,32 @@ String MaterializedPostgreSQLConsumer::advanceLSN(std::shared_ptrsecond; + assert(!table_start_lsn.empty()); + + if (getLSNValue(current_lsn) >= getLSNValue(table_start_lsn)) + { + LOG_TRACE(log, "Synchronization is started for table: {} (start_lsn: {})", relation_name, table_start_lsn); + waiting_list.erase(new_table_with_lsn); + return true; + } + } + + auto skipped_table_with_lsn = skip_list.find(relation_id); /// Table is not present in a skip list - allow synchronization. - if (table_with_lsn == skip_list.end()) + if (skipped_table_with_lsn == skip_list.end()) return true; - const auto & table_start_lsn = table_with_lsn->second; + const auto & table_start_lsn = skipped_table_with_lsn->second; /// Table is in a skip list and has not yet received a valid lsn == it has not been reloaded. if (table_start_lsn.empty()) @@ -544,7 +565,7 @@ bool MaterializedPostgreSQLConsumer::isSyncAllowed(Int32 relation_id) LOG_TRACE(log, "Synchronization is resumed for table: {} (start_lsn: {})", relation_id_to_name[relation_id], table_start_lsn); - skip_list.erase(table_with_lsn); + skip_list.erase(skipped_table_with_lsn); return true; } @@ -576,6 +597,34 @@ void MaterializedPostgreSQLConsumer::markTableAsSkipped(Int32 relation_id, const } +void MaterializedPostgreSQLConsumer::addNested(const String & postgres_table_name, StoragePtr nested_storage, const String & table_start_lsn) +{ + /// Cache new pointer to replacingMergeTree table. + storages.emplace(postgres_table_name, nested_storage); + + /// Add new in-memory buffer. + buffers.emplace(postgres_table_name, Buffer(nested_storage)); + + /// Replication consumer will read wall and check for currently processed table whether it is allowed to start applying + /// changed to this table. + waiting_list[postgres_table_name] = table_start_lsn; +} + + +void MaterializedPostgreSQLConsumer::updateNested(const String & table_name, StoragePtr nested_storage, Int32 table_id, const String & table_start_lsn) +{ + /// Cache new pointer to replacingMergeTree table. + storages[table_name] = nested_storage; + + /// Create a new empty buffer (with updated metadata), where data is first loaded before syncing into actual table. + auto & buffer = buffers.find(table_name)->second; + buffer.createEmptyBuffer(nested_storage); + + /// Set start position to valid lsn. Before it was an empty string. Further read for table allowed, if it has a valid lsn. + skip_list[table_id] = table_start_lsn; +} + + /// Read binary changes from replication slot via COPY command (starting from current lsn in a slot). bool MaterializedPostgreSQLConsumer::readFromReplicationSlot() { @@ -625,9 +674,8 @@ bool MaterializedPostgreSQLConsumer::readFromReplicationSlot() tryLogCurrentException(__PRETTY_FUNCTION__); return false; } - catch (const pqxx::broken_connection & e) + catch (const pqxx::broken_connection &) { - LOG_ERROR(log, "Connection error: {}", e.what()); connection->tryUpdateConnection(); return false; } @@ -641,6 +689,7 @@ bool MaterializedPostgreSQLConsumer::readFromReplicationSlot() if (error_message.find("out of relcache_callback_list slots") == std::string::npos) tryLogCurrentException(__PRETTY_FUNCTION__); + connection->tryUpdateConnection(); return false; } catch (const pqxx::conversion_error & e) @@ -704,17 +753,4 @@ bool MaterializedPostgreSQLConsumer::consume(std::vectorsecond; - buffer.createEmptyBuffer(nested_storage); - - /// Set start position to valid lsn. Before it was an empty string. Further read for table allowed, if it has a valid lsn. - skip_list[table_id] = table_start_lsn; -} - } diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h index 8f3224784f1..327c6866760 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h @@ -27,7 +27,8 @@ public: const String & start_lsn, const size_t max_block_size_, bool allow_automatic_update_, - Storages storages_); + Storages storages_, + const String & name_for_logger); bool consume(std::vector> & skipped_tables); @@ -35,6 +36,8 @@ public: /// process if it was skipped due to schema changes. void updateNested(const String & table_name, StoragePtr nested_storage, Int32 table_id, const String & table_start_lsn); + void addNested(const String & postgres_table_name, StoragePtr nested_storage, const String & table_start_lsn); + private: /// Read approximarely up to max_block_size changes from WAL. bool readFromReplicationSlot(); @@ -45,7 +48,7 @@ private: void processReplicationMessage(const char * replication_message, size_t size); - bool isSyncAllowed(Int32 relation_id); + bool isSyncAllowed(Int32 relation_id, const String & relation_name); struct Buffer { @@ -111,9 +114,12 @@ private: String table_to_insert; /// List of tables which need to be synced after last replication stream. + /// Holds `postgres_table_name` set. std::unordered_set tables_to_sync; + /// `postgres_table_name` -> ReplacingMergeTree table. Storages storages; + /// `postgres_table_name` -> In-memory buffer. Buffers buffers; std::unordered_map relation_id_to_name; @@ -133,6 +139,7 @@ private: /// if relation definition has changed since the last relation definition message. std::unordered_map schema_data; + /// `postgres_relation_id` -> `start_lsn` /// skip_list contains relation ids for tables on which ddl was performed, which can break synchronization. /// This breaking changes are detected in replication stream in according replication message and table is added to skip list. /// After it is finished, a temporary replication slot is created with 'export snapshot' option, and start_lsn is returned. @@ -142,5 +149,13 @@ private: /// No needed message, related to reloaded table will be missed, because messages are not consumed in the meantime, /// i.e. we will not miss the first start_lsn position for reloaded table. std::unordered_map skip_list; + + /// `postgres_table_name` -> `start_lsn` + /// For dynamically added tables. A new table is loaded via snapshot and we get a start lsn position. + /// Once consumer reaches this position, it starts applying replication messages to this table. + /// Inside replication handler we have to ensure that replication consumer does not read data from wal + /// while the process of adding a table to replication is not finished, + /// because we might go beyond this start lsn position before consumer knows that a new table was added. + std::unordered_map waiting_list; }; } diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index 3477397adb7..55864305c8f 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -11,18 +11,20 @@ #include #include #include +#include namespace DB { -static const auto RESCHEDULE_MS = 500; +static const auto RESCHEDULE_MS = 1000; static const auto BACKOFF_TRESHOLD_MS = 10000; namespace ErrorCodes { extern const int LOGICAL_ERROR; extern const int BAD_ARGUMENTS; + extern const int POSTGRESQL_REPLICATION_INTERNAL_ERROR; } PostgreSQLReplicationHandler::PostgreSQLReplicationHandler( @@ -36,7 +38,9 @@ PostgreSQLReplicationHandler::PostgreSQLReplicationHandler( bool allow_automatic_update_, bool is_materialized_postgresql_database_, const String tables_list_) - : log(&Poco::Logger::get("PostgreSQLReplicationHandler")) + : log(&Poco::Logger::get("PostgreSQLReplicationHandler(" + + (is_materialized_postgresql_database_ ? remote_database_name_ : remote_database_name_ + '.' + tables_list_) + + ")")) , context(context_) , is_attach(is_attach_) , remote_database_name(remote_database_name_) @@ -46,7 +50,6 @@ PostgreSQLReplicationHandler::PostgreSQLReplicationHandler( , allow_automatic_update(allow_automatic_update_) , is_materialized_postgresql_database(is_materialized_postgresql_database_) , tables_list(tables_list_) - , connection(std::make_shared(connection_info_)) , milliseconds_to_wait(RESCHEDULE_MS) { replication_slot = fmt::format("{}_ch_replication_slot", replication_identifier); @@ -73,7 +76,8 @@ void PostgreSQLReplicationHandler::waitConnectionAndStart() { try { - connection->connect(); /// Will throw pqxx::broken_connection if no connection at the moment + postgres::Connection connection(connection_info); + connection.connect(); /// Will throw pqxx::broken_connection if no connection at the moment startSynchronization(false); } catch (const pqxx::broken_connection & pqxx_error) @@ -98,14 +102,9 @@ void PostgreSQLReplicationHandler::shutdown() void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) { - { - pqxx::work tx(connection->getRef()); - createPublicationIfNeeded(tx); - tx.commit(); - } - postgres::Connection replication_connection(connection_info, /* replication */true); pqxx::nontransaction tx(replication_connection.getRef()); + createPublicationIfNeeded(tx); /// List of nested tables (table_name -> nested_storage), which is passed to replication consumer. std::unordered_map nested_storages; @@ -116,18 +115,21 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) /// 2. if replication slot already exist, start_lsn is read from pg_replication_slots as /// `confirmed_flush_lsn` - the address (LSN) up to which the logical slot's consumer has confirmed receiving data. /// Data older than this is not available anymore. - /// TODO: more tests String snapshot_name, start_lsn; auto initial_sync = [&]() { createReplicationSlot(tx, start_lsn, snapshot_name); + /// Loading tables from snapshot requires a certain transaction type, so we need to open a new transactin. + /// But we cannot have more than one open transaciton on the same connection. Therefore we have + /// a separate connection to load tables. + postgres::Connection tmp_connection(connection_info); for (const auto & [table_name, storage] : materialized_storages) { try { - nested_storages[table_name] = loadFromSnapshot(snapshot_name, table_name, storage->as ()); + nested_storages[table_name] = loadFromSnapshot(tmp_connection, snapshot_name, table_name, storage->as ()); } catch (Exception & e) { @@ -164,6 +166,7 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) auto * materialized_storage = storage->as (); try { + /// TODO: THIS IS INCORRENT, we might get here if there is no nested, need to check and reload. /// Try load nested table, set materialized table metadata. nested_storages[table_name] = materialized_storage->prepare(); } @@ -187,13 +190,14 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) /// Handler uses it only for loadFromSnapshot and shutdown methods. consumer = std::make_shared( context, - connection, + std::make_shared(connection_info), replication_slot, publication_name, start_lsn, max_block_size, allow_automatic_update, - nested_storages); + nested_storages, + (is_materialized_postgresql_database ? remote_database_name : remote_database_name + '.' + tables_list)); consumer_task->activateAndSchedule(); @@ -202,10 +206,31 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) } -StoragePtr PostgreSQLReplicationHandler::loadFromSnapshot(String & snapshot_name, const String & table_name, +void PostgreSQLReplicationHandler::addStructureToMaterializedStorage(StorageMaterializedPostgreSQL * storage, const String & table_name, ASTPtr database_def) +{ + postgres::Connection connection(connection_info); + pqxx::nontransaction tx(connection.getRef()); + auto table_structure = std::make_unique(fetchPostgreSQLTableStructure(tx, table_name, true, true, true)); + + auto engine = std::make_shared(); + engine->name = "MaterializedPostgreSQL"; + engine->arguments = args; + + auto ast_storage = std::make_shared(); + storage->set(storage->engine, engine); + + auto storage_def = storage->getCreateNestedTableQuery(std::move(table_structure)); + ContextMutablePtr local_context = Context::createCopy(context); + auto table = createTableFromAST(*assert_cast(storage_def.get()), remote_database_name, "", local_context, false).second; + + storage->setInMemoryMetadata(table->getInMemoryMetadata()); +} + + +StoragePtr PostgreSQLReplicationHandler::loadFromSnapshot(postgres::Connection & connection, String & snapshot_name, const String & table_name, StorageMaterializedPostgreSQL * materialized_storage) { - auto tx = std::make_shared(connection->getRef()); + auto tx = std::make_shared(connection.getRef()); std::string query_str = fmt::format("SET TRANSACTION SNAPSHOT '{}'", snapshot_name); tx->exec(query_str); @@ -290,7 +315,7 @@ void PostgreSQLReplicationHandler::consumerFunc() } -bool PostgreSQLReplicationHandler::isPublicationExist(pqxx::work & tx) +bool PostgreSQLReplicationHandler::isPublicationExist(pqxx::nontransaction & tx) { std::string query_str = fmt::format("SELECT exists (SELECT 1 FROM pg_publication WHERE pubname = '{}')", publication_name); pqxx::result result{tx.exec(query_str)}; @@ -299,7 +324,7 @@ bool PostgreSQLReplicationHandler::isPublicationExist(pqxx::work & tx) } -void PostgreSQLReplicationHandler::createPublicationIfNeeded(pqxx::work & tx) +void PostgreSQLReplicationHandler::createPublicationIfNeeded(pqxx::nontransaction & tx) { auto publication_exists = isPublicationExist(tx); @@ -310,7 +335,7 @@ void PostgreSQLReplicationHandler::createPublicationIfNeeded(pqxx::work & tx) "Publication {} already exists, but it is a CREATE query, not ATTACH. Publication will be dropped", publication_name); - connection->execWithRetry([&](pqxx::nontransaction & tx_){ dropPublication(tx_); }); + dropPublication(tx); } if (!is_attach || !publication_exists) @@ -389,7 +414,7 @@ void PostgreSQLReplicationHandler::createReplicationSlot( pqxx::result result{tx.exec(query_str)}; start_lsn = result[0][1].as(); snapshot_name = result[0][2].as(); - LOG_TRACE(log, "Created replication slot: {}, start lsn: {}", replication_slot, start_lsn); + LOG_TRACE(log, "Created replication slot: {}, start lsn: {}, snapshot: {}", replication_slot, start_lsn, snapshot_name); } catch (Exception & e) { @@ -422,22 +447,39 @@ void PostgreSQLReplicationHandler::dropPublication(pqxx::nontransaction & tx) } +void PostgreSQLReplicationHandler::addTableToPublication(pqxx::nontransaction & ntx, const String & table_name) +{ + std::string query_str = fmt::format("ALTER PUBLICATION {} ADD TABLE ONLY {}", publication_name, doubleQuoteString(table_name)); + ntx.exec(query_str); + LOG_TRACE(log, "Added table `{}` to publication `{}`", table_name, publication_name); +} + + +void PostgreSQLReplicationHandler::removeTableFromPublication(pqxx::nontransaction & ntx, const String & table_name) +{ + std::string query_str = fmt::format("ALTER PUBLICATION {} DROP TABLE ONLY {}", publication_name, doubleQuoteString(table_name)); + ntx.exec(query_str); + LOG_TRACE(log, "Removed table `{}` from publication `{}`", table_name, publication_name); +} + + void PostgreSQLReplicationHandler::shutdownFinal() { try { shutdown(); - connection->execWithRetry([&](pqxx::nontransaction & tx){ dropPublication(tx); }); + postgres::Connection connection(connection_info); + connection.execWithRetry([&](pqxx::nontransaction & tx){ dropPublication(tx); }); String last_committed_lsn; - connection->execWithRetry([&](pqxx::nontransaction & tx) + connection.execWithRetry([&](pqxx::nontransaction & tx) { if (isReplicationSlotExist(tx, last_committed_lsn, /* temporary */false)) dropReplicationSlot(tx, /* temporary */false); }); - connection->execWithRetry([&](pqxx::nontransaction & tx) + connection.execWithRetry([&](pqxx::nontransaction & tx) { if (isReplicationSlotExist(tx, last_committed_lsn, /* temporary */true)) dropReplicationSlot(tx, /* temporary */true); @@ -453,12 +495,17 @@ void PostgreSQLReplicationHandler::shutdownFinal() /// Used by MaterializedPostgreSQL database engine. -NameSet PostgreSQLReplicationHandler::fetchRequiredTables(postgres::Connection & connection_) +NameSet PostgreSQLReplicationHandler::fetchRequiredTables() { - pqxx::work tx(connection_.getRef()); + postgres::Connection connection(connection_info); NameSet result_tables; + bool publication_exists_before_startup; + + { + pqxx::nontransaction tx(connection.getRef()); + publication_exists_before_startup = isPublicationExist(tx); + } - bool publication_exists_before_startup = isPublicationExist(tx); LOG_DEBUG(log, "Publication exists: {}, is attach: {}", publication_exists_before_startup, is_attach); Strings expected_tables; @@ -479,7 +526,7 @@ NameSet PostgreSQLReplicationHandler::fetchRequiredTables(postgres::Connection & "Publication {} already exists, but it is a CREATE query, not ATTACH. Publication will be dropped", publication_name); - connection->execWithRetry([&](pqxx::nontransaction & tx_){ dropPublication(tx_); }); + connection.execWithRetry([&](pqxx::nontransaction & tx_){ dropPublication(tx_); }); } else { @@ -489,13 +536,20 @@ NameSet PostgreSQLReplicationHandler::fetchRequiredTables(postgres::Connection & "Publication {} already exists and tables list is empty. Assuming publication is correct.", publication_name); - result_tables = fetchPostgreSQLTablesList(tx, postgres_schema); + { + pqxx::nontransaction tx(connection.getRef()); + result_tables = fetchPostgreSQLTablesList(tx, postgres_schema); + } } /// Check tables list from publication is the same as expected tables list. /// If not - drop publication and return expected tables list. else { - result_tables = fetchTablesFromPublication(tx); + { + pqxx::work tx(connection.getRef()); + result_tables = fetchTablesFromPublication(tx); + } + NameSet diff; std::set_symmetric_difference(expected_tables.begin(), expected_tables.end(), result_tables.begin(), result_tables.end(), @@ -514,7 +568,7 @@ NameSet PostgreSQLReplicationHandler::fetchRequiredTables(postgres::Connection & "Publication {} already exists, but specified tables list differs from publication tables list in tables: {}.", publication_name, diff_tables); - connection->execWithRetry([&](pqxx::nontransaction & tx_){ dropPublication(tx_); }); + connection.execWithRetry([&](pqxx::nontransaction & tx_){ dropPublication(tx_); }); } } } @@ -531,11 +585,13 @@ NameSet PostgreSQLReplicationHandler::fetchRequiredTables(postgres::Connection & /// Fetch all tables list from database. Publication does not exist yet, which means /// that no replication took place. Publication will be created in /// startSynchronization method. - result_tables = fetchPostgreSQLTablesList(tx, postgres_schema); + { + pqxx::nontransaction tx(connection.getRef()); + result_tables = fetchPostgreSQLTablesList(tx, postgres_schema); + } } } - tx.commit(); return result_tables; } @@ -562,6 +618,57 @@ PostgreSQLTableStructurePtr PostgreSQLReplicationHandler::fetchTableStructure( } +void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPostgreSQL * materialized_storage, const String & postgres_table_name) +{ + /// Note: we have to ensure that replication consumer task is stopped when we reload table, because otherwise + /// it can read wal beyond start lsn position (from which this table is being loaded), which will result in loosing data. + /// Therefore we wait here for it to finish current reading stream. We have to wait, because we cannot return OK to client right now. + consumer_task->deactivate(); + + try + { + postgres::Connection replication_connection(connection_info, /* replication */true); + String snapshot_name, start_lsn; + StoragePtr nested_storage; + + { + pqxx::nontransaction tx(replication_connection.getRef()); + auto table_structure = std::make_unique(fetchPostgreSQLTableStructure(tx, postgres_table_name, true, true, true)); + + if (isReplicationSlotExist(tx, start_lsn, /* temporary */true)) + dropReplicationSlot(tx, /* temporary */true); + createReplicationSlot(tx, start_lsn, snapshot_name, /* temporary */true); + + { + postgres::Connection tmp_connection(connection_info); + nested_storage = loadFromSnapshot(tmp_connection, snapshot_name, postgres_table_name, materialized_storage->as ()); + } + auto nested_table_id = nested_storage->getStorageID(); + materialized_storage->setNestedStorageID(nested_table_id); + nested_storage = materialized_storage->prepare(); + } + + { + pqxx::nontransaction tx(replication_connection.getRef()); + addTableToPublication(tx, postgres_table_name); + } + + /// Pass storage to consumer and lsn position, from which to start receiving replication messages for this table. + consumer->addNested(postgres_table_name, nested_storage, start_lsn); + } + catch (...) + { + consumer_task->scheduleAfter(RESCHEDULE_MS); + + auto error_message = getCurrentExceptionMessage(false); + throw Exception(ErrorCodes::POSTGRESQL_REPLICATION_INTERNAL_ERROR, + "Failed to add table `{}` to replication. Info: {}", postgres_table_name, error_message); + } + + consumer_task->schedule(); +} + + void PostgreSQLReplicationHandler::reloadFromSnapshot(const std::vector> & relation_data) { /// If table schema has changed, the table stops consuming changes from replication stream. @@ -579,6 +686,7 @@ void PostgreSQLReplicationHandler::reloadFromSnapshot(const std::vectorcreateTemporary(); /// This snapshot is valid up to the end of the transaction, which exported it. - StoragePtr temp_nested_storage = loadFromSnapshot(snapshot_name, table_name, + StoragePtr temp_nested_storage = loadFromSnapshot(tmp_connection, snapshot_name, table_name, temp_materialized_storage->as ()); auto table_id = materialized_storage->getNestedStorageID(); diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h index 3a0bedc0852..37ea6b2cbea 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h @@ -8,11 +8,6 @@ namespace DB { -/// IDEA: There is ALTER PUBLICATION command to dynamically add and remove tables for replicating (the command is transactional). -/// (Probably, if in a replication stream comes a relation name, which does not currently -/// exist in CH, it can be loaded via snapshot while stream is stopped and then comparing wal positions with -/// current lsn and table start lsn. - class StorageMaterializedPostgreSQL; class PostgreSQLReplicationHandler @@ -43,24 +38,32 @@ public: void addStorage(const std::string & table_name, StorageMaterializedPostgreSQL * storage); /// Fetch list of tables which are going to be replicated. Used for database engine. - NameSet fetchRequiredTables(postgres::Connection & connection_); + NameSet fetchRequiredTables(); /// Start replication setup immediately. void startSynchronization(bool throw_on_error); + void addTableToReplication(StorageMaterializedPostgreSQL * storage, const String & postgres_table_name); + + void addStructureToMaterializedStorage(StorageMaterializedPostgreSQL * storage, const String & table_name); + private: using MaterializedStorages = std::unordered_map; /// Methods to manage Publication. - bool isPublicationExist(pqxx::work & tx); + bool isPublicationExist(pqxx::nontransaction & tx); - void createPublicationIfNeeded(pqxx::work & tx); + void createPublicationIfNeeded(pqxx::nontransaction & tx); NameSet fetchTablesFromPublication(pqxx::work & tx); void dropPublication(pqxx::nontransaction & ntx); + void addTableToPublication(pqxx::nontransaction & ntx, const String & table_name); + + void removeTableFromPublication(pqxx::nontransaction & ntx, const String & table_name); + /// Methods to manage Replication Slots. bool isReplicationSlotExist(pqxx::nontransaction & tx, String & start_lsn, bool temporary = false); @@ -75,7 +78,7 @@ private: void consumerFunc(); - StoragePtr loadFromSnapshot(std::string & snapshot_name, const String & table_name, StorageMaterializedPostgreSQL * materialized_storage); + StoragePtr loadFromSnapshot(postgres::Connection & connection, std::string & snapshot_name, const String & table_name, StorageMaterializedPostgreSQL * materialized_storage); void reloadFromSnapshot(const std::vector> & relation_data); @@ -110,9 +113,6 @@ private: String replication_slot, publication_name; - /// Shared between replication_consumer and replication_handler, but never accessed concurrently. - std::shared_ptr connection; - /// Replication consumer. Manages decoding of replication stream and syncing into tables. std::shared_ptr consumer; diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp index e24e252bf01..5ff00f9babb 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp @@ -51,6 +51,7 @@ StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL( std::unique_ptr replication_settings) : IStorage(table_id_) , WithContext(context_->getGlobalContext()) + , log(&Poco::Logger::get("StorageMaterializedPostgreSQL(" + postgres::formatNameForLogs(remote_database_name, remote_table_name) + ")")) , is_materialized_postgresql_database(false) , has_nested(false) , nested_context(makeNestedTableContext(context_->getGlobalContext())) @@ -72,19 +73,26 @@ StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL( getContext(), is_attach, replication_settings->materialized_postgresql_max_block_size.value, - /* allow_automatic_update */ false, /* is_materialized_postgresql_database */false); + /* allow_automatic_update */ false, /* is_materialized_postgresql_database */false, + remote_table_name_); } /// For the case of MaterializePosgreSQL database engine. /// It is used when nested ReplacingMergeeTree table has not yet be created by replication thread. /// In this case this storage can't be used for read queries. -StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL(const StorageID & table_id_, ContextPtr context_) +StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL( + const StorageID & table_id_, + ContextPtr context_, + const String & postgres_database_name, + const String & postgres_table_name) : IStorage(table_id_) , WithContext(context_->getGlobalContext()) + , log(&Poco::Logger::get("StorageMaterializedPostgreSQL(" + postgres::formatNameForLogs(postgres_database_name, postgres_table_name) + ")")) , is_materialized_postgresql_database(true) , has_nested(false) , nested_context(makeNestedTableContext(context_->getGlobalContext())) + , nested_table_id(table_id_) { } @@ -92,9 +100,14 @@ StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL(const StorageID & t /// Constructor for MaterializedPostgreSQL table engine - for the case of MaterializePosgreSQL database engine. /// It is used when nested ReplacingMergeeTree table has already been created by replication thread. /// This storage is ready to handle read queries. -StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL(StoragePtr nested_storage_, ContextPtr context_) +StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL( + StoragePtr nested_storage_, + ContextPtr context_, + const String & postgres_database_name, + const String & postgres_table_name) : IStorage(nested_storage_->getStorageID()) , WithContext(context_->getGlobalContext()) + , log(&Poco::Logger::get("StorageMaterializedPostgreSQL(" + postgres::formatNameForLogs(postgres_database_name, postgres_table_name) + ")")) , is_materialized_postgresql_database(true) , has_nested(true) , nested_context(makeNestedTableContext(context_->getGlobalContext())) @@ -120,7 +133,7 @@ StoragePtr StorageMaterializedPostgreSQL::createTemporary() const } auto new_context = Context::createCopy(context); - return StorageMaterializedPostgreSQL::create(tmp_table_id, new_context); + return StorageMaterializedPostgreSQL::create(tmp_table_id, new_context, "", table_id.table_name); } @@ -163,6 +176,7 @@ void StorageMaterializedPostgreSQL::createNestedIfNeeded(PostgreSQLTableStructur const auto ast_create = getCreateNestedTableQuery(std::move(table_structure)); auto table_id = getStorageID(); auto tmp_nested_table_id = StorageID(table_id.database_name, getNestedTableName()); + LOG_DEBUG(log, "Creating clickhouse table for postgresql table {}", table_id.getNameForLogs()); try { diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h index becb4f6ba10..62bc6ed713f 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h @@ -49,7 +49,6 @@ namespace DB * * All database methods, apart from tryGetTable(), are devoted only to nested table. * NOTE: It makes sense to allow rename method for MaterializedPostgreSQL table via database method. - * TODO: Make sure replication-to-table data channel is done only by relation_id. * * Also main table has the same InMemoryMetadata as its nested table, so if metadata of nested table changes - main table also has * to update its metadata, because all read requests are passed to MaterializedPostgreSQL table and then it redirects read @@ -57,7 +56,7 @@ namespace DB * * When there is a need to update table structure, there will be created a new MaterializedPostgreSQL table with its own nested table, * it will have updated table schema and all data will be loaded from scratch in the background, while previous table with outadted table - * structure will still serve read requests. When data is loaded, nested tables will be swapped, metadata of metarialzied table will be + * structure will still serve read requests. When data is loaded, nested tables will be swapped, metadata of materialized table will be * updated according to nested table. * **/ @@ -67,9 +66,11 @@ class StorageMaterializedPostgreSQL final : public shared_ptr_helper; public: - StorageMaterializedPostgreSQL(const StorageID & table_id_, ContextPtr context_); + StorageMaterializedPostgreSQL(const StorageID & table_id_, ContextPtr context_, + const String & postgres_database_name, const String & postgres_table_name); - StorageMaterializedPostgreSQL(StoragePtr nested_storage_, ContextPtr context_); + StorageMaterializedPostgreSQL(StoragePtr nested_storage_, ContextPtr context_, + const String & postgres_database_name, const String & postgres_table_name); String getName() const override { return "MaterializedPostgreSQL"; } @@ -123,6 +124,8 @@ public: bool supportsFinal() const override { return true; } + ASTPtr getCreateNestedTableQuery(PostgreSQLTableStructurePtr table_structure); + protected: StorageMaterializedPostgreSQL( const StorageID & table_id_, @@ -140,10 +143,10 @@ private: ASTPtr getColumnDeclaration(const DataTypePtr & data_type) const; - ASTPtr getCreateNestedTableQuery(PostgreSQLTableStructurePtr table_structure); - String getNestedTableName() const; + Poco::Logger * log; + /// Not nullptr only for single MaterializedPostgreSQL storage, because for MaterializedPostgreSQL /// database engine there is one replication handler for all tables. std::unique_ptr replication_handler; diff --git a/src/Storages/StorageInMemoryMetadata.h b/src/Storages/StorageInMemoryMetadata.h index 9accdb9b3b6..66ad2b0254c 100644 --- a/src/Storages/StorageInMemoryMetadata.h +++ b/src/Storages/StorageInMemoryMetadata.h @@ -41,7 +41,7 @@ struct StorageInMemoryMetadata TTLColumnsDescription column_ttls_by_name; /// TTL expressions for table (Move and Rows) TTLTableDescription table_ttl; - /// SETTINGS expression. Supported for MergeTree, Buffer and Kafka. + /// SETTINGS expression. Supported for MergeTree, Buffer, Kafka, RabbitMQ. ASTPtr settings_changes; /// SELECT QUERY. Supported for MaterializedView and View (have to support LiveView). SelectQueryDescription select; From 174340074cbd8181179e1bdd9f4141aeb8881e62 Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 27 Aug 2021 15:50:45 +0300 Subject: [PATCH 03/13] Dynamically add tables complete --- src/Common/ErrorCodes.cpp | 11 +- src/Core/PostgreSQL/Connection.cpp | 3 +- src/Core/PostgreSQL/Utils.cpp | 7 +- src/Databases/DatabaseAtomic.cpp | 7 +- .../DatabaseMaterializedPostgreSQL.cpp | 148 +++++++++--------- .../DatabaseMaterializedPostgreSQL.h | 4 + src/Interpreters/DatabaseCatalog.cpp | 5 +- src/Interpreters/InterpreterAlterQuery.cpp | 8 +- src/Parsers/ASTAlterQuery.cpp | 5 + src/Parsers/ASTAlterQuery.h | 1 - src/Parsers/ParserAlterQuery.cpp | 4 - .../MaterializedPostgreSQLConsumer.cpp | 8 +- .../PostgreSQLReplicationHandler.cpp | 30 ++-- .../PostgreSQL/PostgreSQLReplicationHandler.h | 5 +- .../StorageMaterializedPostgreSQL.cpp | 10 +- .../test.py | 82 ++++++++++ 16 files changed, 213 insertions(+), 125 deletions(-) diff --git a/src/Common/ErrorCodes.cpp b/src/Common/ErrorCodes.cpp index 36d0fafdb4c..681319efe5e 100644 --- a/src/Common/ErrorCodes.cpp +++ b/src/Common/ErrorCodes.cpp @@ -577,9 +577,14 @@ M(606, BACKUP_IS_EMPTY) \ M(607, BACKUP_ELEMENT_DUPLICATE) \ M(608, CANNOT_RESTORE_TABLE) \ - M(609, POSTGRESQL_CONNECTION_FAILURE) \ - M(610, POSTGRESQL_REPLICATION_INTERNAL_ERROR) \ - M(611, QUERY_NOT_ALLOWED) \ + M(609, FUNCTION_ALREADY_EXISTS) \ + M(610, CANNOT_DROP_SYSTEM_FUNCTION) \ + M(611, CANNOT_CREATE_RECURSIVE_FUNCTION) \ + M(612, OBJECT_ALREADY_STORED_ON_DISK) \ + M(613, OBJECT_WAS_NOT_STORED_ON_DISK) \ + M(614, POSTGRESQL_CONNECTION_FAILURE) \ + M(615, POSTGRESQL_REPLICATION_INTERNAL_ERROR) \ + M(616, QUERY_NOT_ALLOWED) \ \ M(999, KEEPER_EXCEPTION) \ M(1000, POCO_EXCEPTION) \ diff --git a/src/Core/PostgreSQL/Connection.cpp b/src/Core/PostgreSQL/Connection.cpp index 2cb52fcff81..8e127afb1db 100644 --- a/src/Core/PostgreSQL/Connection.cpp +++ b/src/Core/PostgreSQL/Connection.cpp @@ -1,9 +1,8 @@ #include "Connection.h" #if USE_LIBPQXX - #include -#include + namespace postgres { diff --git a/src/Core/PostgreSQL/Utils.cpp b/src/Core/PostgreSQL/Utils.cpp index 98cd706be69..accc3b29a93 100644 --- a/src/Core/PostgreSQL/Utils.cpp +++ b/src/Core/PostgreSQL/Utils.cpp @@ -21,11 +21,12 @@ ConnectionInfo formatConnectionString(String dbname, String host, UInt16 port, S String formatNameForLogs(const String & postgres_database_name, const String & postgres_table_name) { - if (postgres_database_name.empty()) - return postgres_table_name; + /// Logger for StorageMaterializedPostgreSQL - both db and table names. + /// Logger for PostgreSQLReplicationHandler and Consumer - either both db and table names or only db name. + assert(!postgres_database_name.empty()); if (postgres_table_name.empty()) return postgres_database_name; - return fmt::format("{}.{}", postgres_database_name, postgres_table_name); + return postgres_database_name + '.' + postgres_table_name; } } diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 8ce17198d3d..5102c924140 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -578,11 +578,11 @@ void DatabaseAtomic::modifySettings(const SettingsChanges & settings_changes, Co create.uuid = getUUID(); create.if_not_exists = false; create.storage = assert_cast(storage_def.get()); - auto * ast_set_query = create.storage->settings; + auto * settings = create.storage->settings; - if (ast_set_query) + if (settings) { - auto & previous_settings = ast_set_query->changes; + auto & previous_settings = settings->changes; for (const auto & change : settings_changes) { auto it = std::find_if(previous_settings.begin(), previous_settings.end(), @@ -614,7 +614,6 @@ void DatabaseAtomic::modifySettings(const SettingsChanges & settings_changes, Co fs::path metadata_file_tmp_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql.tmp"); fs::path metadata_file_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql"); - /// Exclusive flag guarantees, that database is not created right now in another thread. WriteBufferFromFile out(metadata_file_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); writeString(statement, out); diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index b4b0037ff1b..4575a1d7270 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -5,6 +5,9 @@ #include #include +#include +#include +#include #include #include #include @@ -21,9 +24,6 @@ #include #include #include -#include -#include - namespace DB { @@ -145,10 +145,20 @@ void DatabaseMaterializedPostgreSQL::applySettings(const SettingsChanges & setti if (!settings->has(change.name)) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine {} does not support setting `{}`", getEngineName(), change.name); - if (change.name == "materialized_postgresql_tables_list") + if ((change.name == "materialized_postgresql_tables_list")) { - if (local_context->isInternalQuery() || materialized_tables.empty()) - throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "Changin settings `{}` is allowed only internally. Use CREATE TABLE query", change.name); + if (!local_context->isInternalQuery()) + throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "Changing setting `{}` is not allowed", change.name); + } + else if (change.name == "materialized_postgresql_allow_automatic_update") + { + } + else if (change.name == "materialized_postgresql_max_block_size") + { + } + else + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown setting"); } settings->applyChange(change); @@ -192,7 +202,44 @@ void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const return; } - throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "CREATE TABLE is not allowed for database engine {}", getEngineName()); + const auto & create = query->as(); + if (!create->attach) + throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "CREATE TABLE is not allowed for database engine {}. Use ATTACH TABLE instead", getEngineName()); + + /// Create ReplacingMergeTree table. + auto query_copy = query->clone(); + auto create_query = assert_cast(query_copy.get()); + create_query->attach = false; + create_query->attach_short_syntax = false; + DatabaseAtomic::createTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, table, query_copy); + + /// Attach MaterializedPostgreSQL table. + attachTable(table_name, table, {}); +} + + +String DatabaseMaterializedPostgreSQL::getTablesList() const +{ + String tables_list; + for (const auto & table : materialized_tables) + { + if (!tables_list.empty()) + tables_list += ','; + tables_list += table.first; + } + return tables_list; +} + + +ASTPtr DatabaseMaterializedPostgreSQL::getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const +{ + if (!local_context->hasQueryContext()) + return DatabaseAtomic::getCreateTableQueryImpl(table_name, local_context, throw_on_error); + + auto storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); + auto ast_storage = replication_handler->getCreateNestedTableQuery(storage.get(), table_name); + assert_cast(ast_storage.get())->uuid = UUIDHelpers::generateV4(); + return ast_storage; } @@ -202,21 +249,34 @@ void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, cons { auto set = std::make_shared(); set->is_standalone = false; + auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; - set->changes = {SettingChange("materialized_postgresql_tables_list", tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name))}; + if (tables_to_replicate.empty()) + tables_to_replicate = getTablesList(); + + /// tables_to_replicate can be empty if postgres database had no tables when this database was created. + set->changes = {SettingChange("materialized_postgresql_tables_list", + tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name))}; auto command = std::make_shared(); command->type = ASTAlterCommand::Type::MODIFY_DATABASE_SETTING; - command->children.emplace_back(std::move(set)); + command->settings_changes = std::move(set); - auto expr = std::make_shared(); - expr->children.push_back(command); + auto command_list = std::make_shared(); + command_list->children.push_back(command); - ASTAlterQuery alter; - alter.alter_object = ASTAlterQuery::AlterObjectType::DATABASE; - alter.children.emplace_back(std::move(expr)); + auto query = std::make_shared(); + auto * alter = query->as(); - auto storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); + alter->alter_object = ASTAlterQuery::AlterObjectType::DATABASE; + alter->database = database_name; + alter->set(alter->command_list, command_list); + + auto current_context = Context::createCopy(getContext()->getGlobalContext()); + current_context->setInternalQuery(true); + InterpreterAlterQuery(query, current_context).execute(); + + auto storage = StorageMaterializedPostgreSQL::create(table, getContext(), remote_database_name, table_name); materialized_tables[table_name] = storage; replication_handler->addTableToReplication(dynamic_cast(storage.get()), table_name); } @@ -267,64 +327,6 @@ DatabaseTablesIteratorPtr DatabaseMaterializedPostgreSQL::getTablesIterator( return DatabaseAtomic::getTablesIterator(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), filter_by_table_name); } -static ASTPtr getColumnDeclaration(const DataTypePtr & data_type) -{ - WhichDataType which(data_type); - - if (which.isNullable()) - return makeASTFunction("Nullable", getColumnDeclaration(typeid_cast(data_type.get())->getNestedType())); - - if (which.isArray()) - return makeASTFunction("Array", getColumnDeclaration(typeid_cast(data_type.get())->getNestedType())); - - return std::make_shared(data_type->getName()); -} - - -ASTPtr DatabaseMaterializedPostgreSQL::getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const -{ - if (!local_context->hasQueryContext()) - return DatabaseAtomic::getCreateTableQueryImpl(table_name, local_context, throw_on_error); - - /// Note: here we make an assumption that table structure will not change between call to this method and to attachTable(). - auto storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); - replication_handler->addStructureToMaterializedStorage(storage.get(), table_name); - - auto create_table_query = std::make_shared(); - auto table_storage_define = storage_def->clone(); - create_table_query->set(create_table_query->storage, table_storage_define); - - auto columns_declare_list = std::make_shared(); - auto columns_expression_list = std::make_shared(); - - columns_declare_list->set(columns_declare_list->columns, columns_expression_list); - create_table_query->set(create_table_query->columns_list, columns_declare_list); - - /// init create query. - auto table_id = storage->getStorageID(); - create_table_query->table = table_id.table_name; - create_table_query->database = table_id.database_name; - - auto metadata_snapshot = storage->getInMemoryMetadataPtr(); - for (const auto & column_type_and_name : metadata_snapshot->getColumns().getAllPhysical()) - { - const auto & column_declaration = std::make_shared(); - column_declaration->name = column_type_and_name.name; - column_declaration->type = getColumnDeclaration(column_type_and_name.type); - columns_expression_list->children.emplace_back(column_declaration); - } - - ASTStorage * ast_storage = table_storage_define->as(); - ASTs storage_children = ast_storage->children; - auto storage_engine_arguments = ast_storage->engine->arguments; - /// Add table_name to engine arguments - storage_engine_arguments->children.insert(storage_engine_arguments->children.begin() + 2, std::make_shared(table_id.table_name)); - - return create_table_query; -} - - - } #endif diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index 3c75af6cc9e..8b47b45bd75 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -66,12 +66,16 @@ public: void shutdown() override; + String getPostgreSQLDatabaseName() const { return remote_database_name; } + protected: ASTPtr getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const override; private: void startSynchronization(); + String getTablesList() const; + bool is_attach; String remote_database_name; postgres::ConnectionInfo connection_info; diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index ee49333a332..104d2a2e5bf 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -30,6 +30,7 @@ #if USE_LIBPQXX # include +# include #endif namespace fs = std::filesystem; @@ -241,7 +242,9 @@ DatabaseAndTable DatabaseCatalog::getTableImpl( #if USE_LIBPQXX if (!context_->isInternalQuery() && (db_and_table.first->getEngineName() == "MaterializedPostgreSQL")) { - db_and_table.second = std::make_shared(std::move(db_and_table.second), getContext(), "", db_and_table.second->getStorageID().table_name); + db_and_table.second = std::make_shared(std::move(db_and_table.second), getContext(), + assert_cast(db_and_table.first.get())->getPostgreSQLDatabaseName(), + db_and_table.second->getStorageID().table_name); } #endif diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index 2b4c5f3a8fa..0a40275b4bb 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -43,14 +43,12 @@ InterpreterAlterQuery::InterpreterAlterQuery(const ASTPtr & query_ptr_, ContextP BlockIO InterpreterAlterQuery::execute() { const auto & alter = query_ptr->as(); - std::cerr << "\n\n\n" << query_ptr->dumpTree() << std::endl; if (alter.alter_object == ASTAlterQuery::AlterObjectType::DATABASE) return executeToDatabase(alter); - else if (alter.alter_object == ASTAlterQuery::AlterObjectType::DATABASE) + else if (alter.alter_object == ASTAlterQuery::AlterObjectType::TABLE + || alter.alter_object == ASTAlterQuery::AlterObjectType::LIVE_VIEW) return executeToTable(alter); - else if (alter.alter_object == ASTAlterQuery::AlterObjectType::LIVE_VIEW) - return executeToTable(alter); - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown alter"); + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown alter object type"); } diff --git a/src/Parsers/ASTAlterQuery.cpp b/src/Parsers/ASTAlterQuery.cpp index d0adb2e799f..80a0be6b704 100644 --- a/src/Parsers/ASTAlterQuery.cpp +++ b/src/Parsers/ASTAlterQuery.cpp @@ -494,6 +494,11 @@ void ASTAlterQuery::formatQueryImpl(const FormatSettings & settings, FormatState } settings.ostr << indent_str << backQuoteIfNeed(table); } + else if (alter_object == AlterObjectType::DATABASE && !database.empty()) + { + settings.ostr << indent_str << backQuoteIfNeed(database); + } + formatOnCluster(settings); settings.ostr << settings.nl_or_ws; diff --git a/src/Parsers/ASTAlterQuery.h b/src/Parsers/ASTAlterQuery.h index d99bac80fed..4f5cff5e0e0 100644 --- a/src/Parsers/ASTAlterQuery.h +++ b/src/Parsers/ASTAlterQuery.h @@ -218,7 +218,6 @@ public: UNKNOWN, }; - // bool is_live_view{false}; /// true for ALTER LIVE VIEW AlterObjectType alter_object = AlterObjectType::UNKNOWN; ASTExpressionList * command_list = nullptr; diff --git a/src/Parsers/ParserAlterQuery.cpp b/src/Parsers/ParserAlterQuery.cpp index e8e0db8c492..da9c533f5cc 100644 --- a/src/Parsers/ParserAlterQuery.cpp +++ b/src/Parsers/ParserAlterQuery.cpp @@ -832,14 +832,11 @@ bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) if (alter_object_type == ASTAlterQuery::AlterObjectType::DATABASE) { - std::cerr << "\n\n\nOK!\n\n"; if (!parseDatabase(pos, expected, query->database)) return false; - std::cerr << "database name: " << query->database << std::endl; } else { - std::cerr << "\n\n\nNOT OK!\n\n"; if (!parseDatabaseAndTableName(pos, expected, query->database, query->table)) return false; @@ -859,7 +856,6 @@ bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) query->set(query->command_list, command_list); query->alter_object = alter_object_type; - std::cerr << "\n\n\nalter query: " << query->dumpTree() << std::endl; return true; } diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp index f9c448e9a5d..62ed7953c72 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp @@ -361,8 +361,6 @@ void MaterializedPostgreSQLConsumer::processReplicationMessage(const char * repl constexpr size_t transaction_commit_timestamp_len = 8; pos += unused_flags_len + commit_lsn_len + transaction_end_lsn_len + transaction_commit_timestamp_len; - // LOG_DEBUG(log, "Current lsn: {} = {}", current_lsn, getLSNValue(current_lsn)); /// Will be removed - final_lsn = current_lsn; break; } @@ -606,7 +604,7 @@ void MaterializedPostgreSQLConsumer::addNested(const String & postgres_table_nam buffers.emplace(postgres_table_name, Buffer(nested_storage)); /// Replication consumer will read wall and check for currently processed table whether it is allowed to start applying - /// changed to this table. + /// changes to this table. waiting_list[postgres_table_name] = table_start_lsn; } @@ -749,7 +747,9 @@ bool MaterializedPostgreSQLConsumer::consume(std::vector(fetchPostgreSQLTableStructure(tx, table_name, true, true, true)); - - auto engine = std::make_shared(); - engine->name = "MaterializedPostgreSQL"; - engine->arguments = args; - - auto ast_storage = std::make_shared(); - storage->set(storage->engine, engine); - - auto storage_def = storage->getCreateNestedTableQuery(std::move(table_structure)); - ContextMutablePtr local_context = Context::createCopy(context); - auto table = createTableFromAST(*assert_cast(storage_def.get()), remote_database_name, "", local_context, false).second; - - storage->setInMemoryMetadata(table->getInMemoryMetadata()); + return storage->getCreateNestedTableQuery(std::move(table_structure)); } @@ -622,26 +610,28 @@ void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPost { /// Note: we have to ensure that replication consumer task is stopped when we reload table, because otherwise /// it can read wal beyond start lsn position (from which this table is being loaded), which will result in loosing data. - /// Therefore we wait here for it to finish current reading stream. We have to wait, because we cannot return OK to client right now. consumer_task->deactivate(); - try { + LOG_TRACE(log, "Adding table `{}` to replication", postgres_table_name); postgres::Connection replication_connection(connection_info, /* replication */true); String snapshot_name, start_lsn; StoragePtr nested_storage; { pqxx::nontransaction tx(replication_connection.getRef()); - auto table_structure = std::make_unique(fetchPostgreSQLTableStructure(tx, postgres_table_name, true, true, true)); - if (isReplicationSlotExist(tx, start_lsn, /* temporary */true)) dropReplicationSlot(tx, /* temporary */true); createReplicationSlot(tx, start_lsn, snapshot_name, /* temporary */true); + /// Protect against deadlock. + auto nested = DatabaseCatalog::instance().tryGetTable(materialized_storage->getNestedStorageID(), materialized_storage->getNestedTableContext()); + if (!nested) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Internal table was not created"); + { postgres::Connection tmp_connection(connection_info); - nested_storage = loadFromSnapshot(tmp_connection, snapshot_name, postgres_table_name, materialized_storage->as ()); + nested_storage = loadFromSnapshot(tmp_connection, snapshot_name, postgres_table_name, materialized_storage); } auto nested_table_id = nested_storage->getStorageID(); materialized_storage->setNestedStorageID(nested_table_id); @@ -655,6 +645,7 @@ void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPost /// Pass storage to consumer and lsn position, from which to start receiving replication messages for this table. consumer->addNested(postgres_table_name, nested_storage, start_lsn); + LOG_TRACE(log, "Table `{}` successfully added to replication", postgres_table_name); } catch (...) { @@ -664,7 +655,6 @@ void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPost throw Exception(ErrorCodes::POSTGRESQL_REPLICATION_INTERNAL_ERROR, "Failed to add table `{}` to replication. Info: {}", postgres_table_name, error_message); } - consumer_task->schedule(); } diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h index 37ea6b2cbea..a0c78b5d425 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h @@ -3,6 +3,7 @@ #include "MaterializedPostgreSQLConsumer.h" #include #include +#include namespace DB @@ -43,9 +44,9 @@ public: /// Start replication setup immediately. void startSynchronization(bool throw_on_error); - void addTableToReplication(StorageMaterializedPostgreSQL * storage, const String & postgres_table_name); + ASTPtr getCreateNestedTableQuery(StorageMaterializedPostgreSQL * storage, const String & table_name); - void addStructureToMaterializedStorage(StorageMaterializedPostgreSQL * storage, const String & table_name); + void addTableToReplication(StorageMaterializedPostgreSQL * storage, const String & postgres_table_name); private: using MaterializedStorages = std::unordered_map; diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp index 5ff00f9babb..1bf3c5ce1ca 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp @@ -173,6 +173,9 @@ StorageID StorageMaterializedPostgreSQL::getNestedStorageID() const void StorageMaterializedPostgreSQL::createNestedIfNeeded(PostgreSQLTableStructurePtr table_structure) { + if (tryGetNested()) + return; + const auto ast_create = getCreateNestedTableQuery(std::move(table_structure)); auto table_id = getStorageID(); auto tmp_nested_table_id = StorageID(table_id.database_name, getNestedTableName()); @@ -477,9 +480,10 @@ void registerStorageMaterializedPostgreSQL(StorageFactory & factory) postgresql_replication_settings->loadFromQuery(*args.storage_def); if (engine_args.size() != 5) - throw Exception("Storage MaterializedPostgreSQL requires 5 parameters: " - "PostgreSQL('host:port', 'database', 'table', 'username', 'password'", - ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); + throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, + "Storage MaterializedPostgreSQL requires 5 parameters: " + "PostgreSQL('host:port', 'database', 'table', 'username', 'password'. Got {}", + engine_args.size()); for (auto & engine_arg : engine_args) engine_arg = evaluateConstantExpressionOrIdentifierAsLiteral(engine_arg, args.getContext()); diff --git a/tests/integration/test_postgresql_replica_database_engine/test.py b/tests/integration/test_postgresql_replica_database_engine/test.py index 40324089b1b..4d407579b52 100644 --- a/tests/integration/test_postgresql_replica_database_engine/test.py +++ b/tests/integration/test_postgresql_replica_database_engine/test.py @@ -923,6 +923,88 @@ def test_abrupt_server_restart_while_heavy_replication(started_cluster): cursor.execute('drop table if exists postgresql_replica_{};'.format(i)) +def test_add_new_table_to_replication(started_cluster): + drop_materialized_db() + conn = get_postgres_conn(ip=started_cluster.postgres_ip, + port=started_cluster.postgres_port, + database=True) + cursor = conn.cursor() + NUM_TABLES = 5 + + for i in range(NUM_TABLES): + create_postgres_table(cursor, 'postgresql_replica_{}'.format(i)); + instance.query("INSERT INTO postgres_database.postgresql_replica_{} SELECT number, {} from numbers(10000)".format(i, i)) + + create_materialized_db(ip=started_cluster.postgres_ip, + port=started_cluster.postgres_port) + + for i in range(NUM_TABLES): + table_name = 'postgresql_replica_{}'.format(i) + check_tables_are_synchronized(table_name); + + result = instance.query("SHOW TABLES FROM test_database") + assert(result == "postgresql_replica_0\npostgresql_replica_1\npostgresql_replica_2\npostgresql_replica_3\npostgresql_replica_4\n") + + table_name = 'postgresql_replica_5' + create_postgres_table(cursor, table_name) + instance.query("INSERT INTO postgres_database.{} SELECT number, number from numbers(10000)".format(table_name)) + + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") # Check without ip + assert(result[-59:] == "\\'postgres_database\\', \\'postgres\\', \\'mysecretpassword\\')\n") + + result = instance.query_and_get_error("ALTER DATABASE test_database MODIFY SETTING materialized_postgresql_tables_list='tabl1'") + assert('Changing setting `materialized_postgresql_tables_list` is not allowed' in result) + + result = instance.query_and_get_error("ALTER DATABASE test_database MODIFY SETTING materialized_postgresql_tables='tabl1'") + assert('Database engine MaterializedPostgreSQL does not support setting' in result) + + instance.query("ATTACH TABLE test_database.{}".format(table_name)); + + result = instance.query("SHOW TABLES FROM test_database") + assert(result == "postgresql_replica_0\npostgresql_replica_1\npostgresql_replica_2\npostgresql_replica_3\npostgresql_replica_4\npostgresql_replica_5\n") + + check_tables_are_synchronized(table_name); + instance.query("INSERT INTO postgres_database.{} SELECT number, number from numbers(10000, 10000)".format(table_name)) + check_tables_are_synchronized(table_name); + + result = instance.query_and_get_error("ATTACH TABLE test_database.{}".format(table_name)); + assert('Table test_database.postgresql_replica_5 already exists' in result) + + result = instance.query_and_get_error("ATTACH TABLE test_database.unknown_table"); + assert('PostgreSQL table unknown_table does not exist' in result) + + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") + assert(result[-180:] == ")\\nSETTINGS materialized_postgresql_tables_list = \\'postgresql_replica_0,postgresql_replica_1,postgresql_replica_2,postgresql_replica_3,postgresql_replica_4,postgresql_replica_5\\'\n") + + table_name = 'postgresql_replica_6' + create_postgres_table(cursor, table_name) + instance.query("INSERT INTO postgres_database.{} SELECT number, number from numbers(10000)".format(table_name)) + instance.query("ATTACH TABLE test_database.{}".format(table_name)); + + instance.restart_clickhouse() + + table_name = 'postgresql_replica_7' + create_postgres_table(cursor, table_name) + instance.query("INSERT INTO postgres_database.{} SELECT number, number from numbers(10000)".format(table_name)) + instance.query("ATTACH TABLE test_database.{}".format(table_name)); + + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") + assert(result[-222:] == ")\\nSETTINGS materialized_postgresql_tables_list = \\'postgresql_replica_0,postgresql_replica_1,postgresql_replica_2,postgresql_replica_3,postgresql_replica_4,postgresql_replica_5,postgresql_replica_6,postgresql_replica_7\\'\n") + + result = instance.query("SHOW TABLES FROM test_database") + assert(result == "postgresql_replica_0\npostgresql_replica_1\npostgresql_replica_2\npostgresql_replica_3\npostgresql_replica_4\npostgresql_replica_5\npostgresql_replica_6\npostgresql_replica_7\n") + + for i in range(NUM_TABLES + 3): + table_name = 'postgresql_replica_{}'.format(i) + check_tables_are_synchronized(table_name); + + for i in range(NUM_TABLES + 3): + cursor.execute('drop table if exists postgresql_replica_{};'.format(i)) + + if __name__ == '__main__': cluster.start() input("Cluster created, press any key to destroy...") From a2b0996ac398ebb66c0cb514c242fc7ff06b612c Mon Sep 17 00:00:00 2001 From: kssenii Date: Sat, 28 Aug 2021 16:42:36 +0300 Subject: [PATCH 04/13] Dynamically remove tables --- .../DatabaseMaterializedPostgreSQL.cpp | 102 ++++++++++++++---- .../DatabaseMaterializedPostgreSQL.h | 10 +- .../MaterializedPostgreSQLConsumer.cpp | 11 ++ .../MaterializedPostgreSQLConsumer.h | 5 + .../PostgreSQLReplicationHandler.cpp | 27 +++++ .../PostgreSQL/PostgreSQLReplicationHandler.h | 2 + .../test.py | 59 ++++++++++ 7 files changed, 192 insertions(+), 24 deletions(-) diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 4575a1d7270..7d378355d64 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -33,6 +33,7 @@ namespace ErrorCodes extern const int NOT_IMPLEMENTED; extern const int LOGICAL_ERROR; extern const int QUERY_NOT_ALLOWED; + extern const int UNKNOWN_TABLE; } DatabaseMaterializedPostgreSQL::DatabaseMaterializedPostgreSQL( @@ -218,13 +219,17 @@ void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const } -String DatabaseMaterializedPostgreSQL::getTablesList() const +String DatabaseMaterializedPostgreSQL::getTablesList(const String & except) const { String tables_list; for (const auto & table : materialized_tables) { + if (table.first == except) + continue; + if (!tables_list.empty()) tables_list += ','; + tables_list += table.first; } return tables_list; @@ -243,38 +248,45 @@ ASTPtr DatabaseMaterializedPostgreSQL::getCreateTableQueryImpl(const String & ta } +ASTPtr DatabaseMaterializedPostgreSQL::createAlterSettingsQuery(const SettingChange & new_setting) +{ + auto set = std::make_shared(); + set->is_standalone = false; + set->changes = {new_setting}; + + auto command = std::make_shared(); + command->type = ASTAlterCommand::Type::MODIFY_DATABASE_SETTING; + command->settings_changes = std::move(set); + + auto command_list = std::make_shared(); + command_list->children.push_back(command); + + auto query = std::make_shared(); + auto * alter = query->as(); + + alter->alter_object = ASTAlterQuery::AlterObjectType::DATABASE; + alter->database = database_name; + alter->set(alter->command_list, command_list); + + return query; +} + + void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, const StoragePtr & table, const String & relative_table_path) { if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) { - auto set = std::make_shared(); - set->is_standalone = false; - auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; if (tables_to_replicate.empty()) tables_to_replicate = getTablesList(); /// tables_to_replicate can be empty if postgres database had no tables when this database was created. - set->changes = {SettingChange("materialized_postgresql_tables_list", - tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name))}; - - auto command = std::make_shared(); - command->type = ASTAlterCommand::Type::MODIFY_DATABASE_SETTING; - command->settings_changes = std::move(set); - - auto command_list = std::make_shared(); - command_list->children.push_back(command); - - auto query = std::make_shared(); - auto * alter = query->as(); - - alter->alter_object = ASTAlterQuery::AlterObjectType::DATABASE; - alter->database = database_name; - alter->set(alter->command_list, command_list); + SettingChange new_setting("materialized_postgresql_tables_list", tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name)); + auto alter_query = createAlterSettingsQuery(new_setting); auto current_context = Context::createCopy(getContext()->getGlobalContext()); current_context->setInternalQuery(true); - InterpreterAlterQuery(query, current_context).execute(); + InterpreterAlterQuery(alter_query, current_context).execute(); auto storage = StorageMaterializedPostgreSQL::create(table, getContext(), remote_database_name, table_name); materialized_tables[table_name] = storage; @@ -287,6 +299,54 @@ void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, cons } +StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name) +{ + if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) + { + auto & table_to_delete = materialized_tables[table_name]; + if (!table_to_delete) + throw Exception(ErrorCodes::UNKNOWN_TABLE, "Materialized table `{}` does not exist", table_name); + + auto tables_to_replicate = getTablesList(table_name); + + /// tables_to_replicate can be empty if postgres database had no tables when this database was created. + SettingChange new_setting("materialized_postgresql_tables_list", tables_to_replicate); + auto alter_query = createAlterSettingsQuery(new_setting); + + { + auto current_context = Context::createCopy(getContext()->getGlobalContext()); + current_context->setInternalQuery(true); + InterpreterAlterQuery(alter_query, current_context).execute(); + } + + auto nested = table_to_delete->as()->getNested(); + if (!nested) + throw Exception(ErrorCodes::UNKNOWN_TABLE, "Inner table `{}` does not exist", table_name); + + replication_handler->removeTableFromReplication(table_name); + + try + { + auto current_context = Context::createCopy(getContext()->getGlobalContext()); + current_context->makeQueryContext(); + DatabaseAtomic::dropTable(current_context, table_name, true); + } + catch (Exception & e) + { + e.addMessage("while removing table `" + table_name + "` from replication"); + throw; + } + + materialized_tables.erase(table_name); + return nullptr; + } + else + { + return DatabaseAtomic::detachTable(table_name); + } +} + + void DatabaseMaterializedPostgreSQL::shutdown() { stopReplication(); diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index 8b47b45bd75..40abb24cccf 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -50,9 +50,11 @@ public: StoragePtr tryGetTable(const String & name, ContextPtr context) const override; - void createTable(ContextPtr context, const String & name, const StoragePtr & table, const ASTPtr & query) override; + void createTable(ContextPtr context, const String & table_name, const StoragePtr & table, const ASTPtr & query) override; - void attachTable(const String & name, const StoragePtr & table, const String & relative_table_path) override; + void attachTable(const String & table_name, const StoragePtr & table, const String & relative_table_path) override; + + StoragePtr detachTable(const String & table_name) override; void dropTable(ContextPtr local_context, const String & name, bool no_delay) override; @@ -74,7 +76,9 @@ protected: private: void startSynchronization(); - String getTablesList() const; + ASTPtr createAlterSettingsQuery(const SettingChange & change); + + String getTablesList(const String & except = {}) const; bool is_attach; String remote_database_name; diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp index 62ed7953c72..131ed066b66 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp @@ -528,6 +528,9 @@ String MaterializedPostgreSQLConsumer::advanceLSN(std::shared_ptr waiting_list; + + /// Since replication may be some time behind, we need to ensure that replication messages for deleted tables are ignored. + std::unordered_set deleted_tables; }; } diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index dc297c9d059..06ad79c5750 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -659,6 +659,33 @@ void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPost } +void PostgreSQLReplicationHandler::removeTableFromReplication(const String & postgres_table_name) +{ + consumer_task->deactivate(); + try + { + postgres::Connection replication_connection(connection_info, /* replication */true); + + { + pqxx::nontransaction tx(replication_connection.getRef()); + removeTableFromPublication(tx, postgres_table_name); + } + + /// Pass storage to consumer and lsn position, from which to start receiving replication messages for this table. + consumer->removeNested(postgres_table_name); + } + catch (...) + { + consumer_task->scheduleAfter(RESCHEDULE_MS); + + auto error_message = getCurrentExceptionMessage(false); + throw Exception(ErrorCodes::POSTGRESQL_REPLICATION_INTERNAL_ERROR, + "Failed to remove table `{}` from replication. Info: {}", postgres_table_name, error_message); + } + consumer_task->schedule(); +} + + void PostgreSQLReplicationHandler::reloadFromSnapshot(const std::vector> & relation_data) { /// If table schema has changed, the table stops consuming changes from replication stream. diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h index a0c78b5d425..047802d10e1 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h @@ -48,6 +48,8 @@ public: void addTableToReplication(StorageMaterializedPostgreSQL * storage, const String & postgres_table_name); + void removeTableFromReplication(const String & postgres_table_name); + private: using MaterializedStorages = std::unordered_map; diff --git a/tests/integration/test_postgresql_replica_database_engine/test.py b/tests/integration/test_postgresql_replica_database_engine/test.py index 4d407579b52..0ccc6ad7f2d 100644 --- a/tests/integration/test_postgresql_replica_database_engine/test.py +++ b/tests/integration/test_postgresql_replica_database_engine/test.py @@ -1005,6 +1005,65 @@ def test_add_new_table_to_replication(started_cluster): cursor.execute('drop table if exists postgresql_replica_{};'.format(i)) +def test_remove_table_from_replication(started_cluster): + drop_materialized_db() + conn = get_postgres_conn(ip=started_cluster.postgres_ip, + port=started_cluster.postgres_port, + database=True) + cursor = conn.cursor() + NUM_TABLES = 5 + + for i in range(NUM_TABLES): + create_postgres_table(cursor, 'postgresql_replica_{}'.format(i)); + instance.query("INSERT INTO postgres_database.postgresql_replica_{} SELECT number, {} from numbers(10000)".format(i, i)) + + create_materialized_db(ip=started_cluster.postgres_ip, + port=started_cluster.postgres_port) + + for i in range(NUM_TABLES): + table_name = 'postgresql_replica_{}'.format(i) + check_tables_are_synchronized(table_name); + + result = instance.query("SHOW TABLES FROM test_database") + assert(result == "postgresql_replica_0\npostgresql_replica_1\npostgresql_replica_2\npostgresql_replica_3\npostgresql_replica_4\n") + + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") + assert(result[-59:] == "\\'postgres_database\\', \\'postgres\\', \\'mysecretpassword\\')\n") + + table_name = 'postgresql_replica_4' + instance.query('DETACH TABLE test_database.{}'.format(table_name)); + result = instance.query_and_get_error('SELECT * FROM test_database.{}'.format(table_name)) + assert("doesn't exist" in result) + + result = instance.query("SHOW TABLES FROM test_database") + assert(result == "postgresql_replica_0\npostgresql_replica_1\npostgresql_replica_2\npostgresql_replica_3\n") + + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") + assert(result[-138:] == ")\\nSETTINGS materialized_postgresql_tables_list = \\'postgresql_replica_0,postgresql_replica_1,postgresql_replica_2,postgresql_replica_3\\'\n") + + instance.query('ATTACH TABLE test_database.{}'.format(table_name)); + check_tables_are_synchronized(table_name); + + for i in range(NUM_TABLES): + table_name = 'postgresql_replica_{}'.format(i) + check_tables_are_synchronized(table_name); + + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") + assert(result[-159:] == ")\\nSETTINGS materialized_postgresql_tables_list = \\'postgresql_replica_0,postgresql_replica_1,postgresql_replica_2,postgresql_replica_3,postgresql_replica_4\\'\n") + + table_name = 'postgresql_replica_1' + instance.query('DETACH TABLE test_database.{}'.format(table_name)); + result = instance.query('SHOW CREATE DATABASE test_database') + assert(result[:63] == "CREATE DATABASE test_database\\nENGINE = MaterializedPostgreSQL(") + assert(result[-138:] == ")\\nSETTINGS materialized_postgresql_tables_list = \\'postgresql_replica_0,postgresql_replica_2,postgresql_replica_3,postgresql_replica_4\\'\n") + + for i in range(NUM_TABLES): + cursor.execute('drop table if exists postgresql_replica_{};'.format(i)) + + if __name__ == '__main__': cluster.start() input("Cluster created, press any key to destroy...") From fd621381c747133cf0f778142dca8f7961117019 Mon Sep 17 00:00:00 2001 From: kssenii Date: Sat, 28 Aug 2021 16:56:39 +0300 Subject: [PATCH 05/13] Allow modify some other settings --- .../PostgreSQL/DatabaseMaterializedPostgreSQL.cpp | 6 ++---- .../PostgreSQL/MaterializedPostgreSQLConsumer.cpp | 10 ++++++++++ .../PostgreSQL/MaterializedPostgreSQLConsumer.h | 5 ++++- .../PostgreSQL/PostgreSQLReplicationHandler.cpp | 8 ++++++++ src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h | 3 +++ 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 7d378355d64..bcd80fe62e7 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -151,11 +151,9 @@ void DatabaseMaterializedPostgreSQL::applySettings(const SettingsChanges & setti if (!local_context->isInternalQuery()) throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "Changing setting `{}` is not allowed", change.name); } - else if (change.name == "materialized_postgresql_allow_automatic_update") - { - } - else if (change.name == "materialized_postgresql_max_block_size") + else if ((change.name == "materialized_postgresql_allow_automatic_update") || (change.name == "materialized_postgresql_max_block_size")) { + replication_handler->setSetting(change); } else { diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp index 131ed066b66..c992699a206 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp @@ -8,6 +8,7 @@ #include #include #include +#include namespace DB @@ -634,6 +635,15 @@ void MaterializedPostgreSQLConsumer::removeNested(const String & postgres_table_ } +void MaterializedPostgreSQLConsumer::setSetting(const SettingChange & setting) +{ + if (setting.name == "materialized_postgresql_max_block_size") + max_block_size = setting.value.safeGet(); + else if (setting.name == "materialized_postgresql_allow_automatic_update") + allow_automatic_update = setting.value.safeGet(); +} + + /// Read binary changes from replication slot via COPY command (starting from current lsn in a slot). bool MaterializedPostgreSQLConsumer::readFromReplicationSlot() { diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h index a7ea337f056..88f4ff4b4da 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.h @@ -13,6 +13,7 @@ namespace DB { +struct SettingChange; class MaterializedPostgreSQLConsumer { @@ -40,6 +41,8 @@ public: void removeNested(const String & postgres_table_name); + void setSetting(const SettingChange & setting); + private: /// Read approximarely up to max_block_size changes from WAL. bool readFromReplicationSlot(); @@ -110,7 +113,7 @@ private: /// current_lsn converted from String to Int64 via getLSNValue(). UInt64 lsn_value; - const size_t max_block_size; + size_t max_block_size; bool allow_automatic_update; String table_to_insert; diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index 06ad79c5750..8974ad93149 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -451,6 +451,14 @@ void PostgreSQLReplicationHandler::removeTableFromPublication(pqxx::nontransacti } +void PostgreSQLReplicationHandler::setSetting(const SettingChange & setting) +{ + consumer_task->deactivate(); + consumer->setSetting(setting); + consumer_task->schedule(); +} + + void PostgreSQLReplicationHandler::shutdownFinal() { try diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h index 047802d10e1..18991b0d561 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.h @@ -10,6 +10,7 @@ namespace DB { class StorageMaterializedPostgreSQL; +struct SettingChange; class PostgreSQLReplicationHandler { @@ -50,6 +51,8 @@ public: void removeTableFromReplication(const String & postgres_table_name); + void setSetting(const SettingChange & setting); + private: using MaterializedStorages = std::unordered_map; From cc90d787f8e91751d61c1227a6f298001d8e775a Mon Sep 17 00:00:00 2001 From: kssenii Date: Sat, 28 Aug 2021 17:30:22 +0300 Subject: [PATCH 06/13] Slightly better --- .../materialized-postgresql.md | 22 ++++++++++++++++++- src/Databases/DatabaseAtomic.cpp | 19 ++++++---------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/docs/en/engines/database-engines/materialized-postgresql.md b/docs/en/engines/database-engines/materialized-postgresql.md index 89c7c803bb3..986396019da 100644 --- a/docs/en/engines/database-engines/materialized-postgresql.md +++ b/docs/en/engines/database-engines/materialized-postgresql.md @@ -23,6 +23,20 @@ ENGINE = MaterializedPostgreSQL('host:port', ['database' | database], 'user', 'p - `user` — PostgreSQL user. - `password` — User password. +## Dynamicaly adding new tables to replication + +``` sql +ATTACH TABLE postgres_database.new_table; +``` + +It will work as well if there is a setting `materialized_postgresql_tables_list`. + +## Dynamicaly removing tables from replication + +``` sql +DETACH TABLE postgres_database.table_to_remove; +``` + ## Settings {#settings} - [materialized_postgresql_max_block_size](../../operations/settings/settings.md#materialized-postgresql-max-block-size) @@ -40,6 +54,12 @@ SETTINGS materialized_postgresql_max_block_size = 65536, SELECT * FROM database1.table1; ``` +It is also possible to change settings at run time. + +``` sql +ALTER DATABASE postgres_database MODIFY SETTING materialized_postgresql_max_block_size = ; +``` + ## Requirements {#requirements} 1. The [wal_level](https://www.postgresql.org/docs/current/runtime-config-wal.html) setting must have a value `logical` and `max_replication_slots` parameter must have a value at least `2` in the PostgreSQL config file. @@ -73,7 +93,7 @@ WHERE oid = 'postgres_table'::regclass; !!! warning "Warning" Replication of [**TOAST**](https://www.postgresql.org/docs/9.5/storage-toast.html) values is not supported. The default value for the data type will be used. - + ## Example of Use {#example-of-use} ``` sql diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 5102c924140..e16db6477d8 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -572,14 +572,9 @@ void DatabaseAtomic::modifySettings(const SettingsChanges & settings_changes, Co { applySettings(settings_changes, local_context); - ASTCreateQuery create; - create.attach = true; - create.database = "_"; - create.uuid = getUUID(); - create.if_not_exists = false; - create.storage = assert_cast(storage_def.get()); - auto * settings = create.storage->settings; - + auto create_query = getCreateDatabaseQuery()->clone(); + auto create = create_query->as(); + auto * settings = create->storage->settings; if (settings) { auto & previous_settings = settings->changes; @@ -598,14 +593,14 @@ void DatabaseAtomic::modifySettings(const SettingsChanges & settings_changes, Co auto settings = std::make_shared(); settings->is_standalone = false; settings->changes = settings_changes; - create.storage->set(create.storage->settings, settings->clone()); + create->storage->set(create->storage->settings, settings->clone()); } - create.attach = true; - create.if_not_exists = false; + create->attach = true; + create->if_not_exists = false; WriteBufferFromOwnString statement_buf; - formatAST(create, statement_buf, false); + formatAST(*create, statement_buf, false); writeChar('\n', statement_buf); String statement = statement_buf.str(); From 378e1f74aa2bdbaf094e691540db17288cfc051e Mon Sep 17 00:00:00 2001 From: kssenii Date: Sat, 28 Aug 2021 19:09:35 +0300 Subject: [PATCH 07/13] Fix tests --- .../en/engines/database-engines/materialized-postgresql.md | 4 ++-- src/Access/AccessType.h | 3 ++- .../PostgreSQL/DatabaseMaterializedPostgreSQL.cpp | 1 + src/Interpreters/InterpreterAlterQuery.cpp | 2 +- src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp | 7 ++++--- tests/queries/0_stateless/01271_show_privileges.reference | 2 ++ 6 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/en/engines/database-engines/materialized-postgresql.md b/docs/en/engines/database-engines/materialized-postgresql.md index 986396019da..2272246a7c5 100644 --- a/docs/en/engines/database-engines/materialized-postgresql.md +++ b/docs/en/engines/database-engines/materialized-postgresql.md @@ -23,7 +23,7 @@ ENGINE = MaterializedPostgreSQL('host:port', ['database' | database], 'user', 'p - `user` — PostgreSQL user. - `password` — User password. -## Dynamicaly adding new tables to replication +## Dynamically adding new tables to replication ``` sql ATTACH TABLE postgres_database.new_table; @@ -31,7 +31,7 @@ ATTACH TABLE postgres_database.new_table; It will work as well if there is a setting `materialized_postgresql_tables_list`. -## Dynamicaly removing tables from replication +## Dynamically removing tables from replication ``` sql DETACH TABLE postgres_database.table_to_remove; diff --git a/src/Access/AccessType.h b/src/Access/AccessType.h index 57342ee5503..823ecebe12d 100644 --- a/src/Access/AccessType.h +++ b/src/Access/AccessType.h @@ -71,9 +71,10 @@ enum class AccessType M(ALTER_FETCH_PARTITION, "ALTER FETCH PART, FETCH PARTITION", TABLE, ALTER_TABLE) \ M(ALTER_FREEZE_PARTITION, "FREEZE PARTITION, UNFREEZE", TABLE, ALTER_TABLE) \ \ - M(ALTER_DATABASE_SETTINGS, "ALTER DATABASE SETTING, ALTER MODIFY DATABASE SETTING, MODIFY DATABASE SETTING", TABLE, ALTER_TABLE) /* allows to execute ALTER MODIFY SETTING */\ + M(ALTER_DATABASE_SETTINGS, "ALTER DATABASE SETTING, ALTER MODIFY DATABASE SETTING, MODIFY DATABASE SETTING", DATABASE, ALTER_DATABASE) /* allows to execute ALTER MODIFY SETTING */\ \ M(ALTER_TABLE, "", GROUP, ALTER) \ + M(ALTER_DATABASE, "", GROUP, ALTER) \ \ M(ALTER_VIEW_REFRESH, "ALTER LIVE VIEW REFRESH, REFRESH VIEW", VIEW, ALTER_VIEW) \ M(ALTER_VIEW_MODIFY_QUERY, "ALTER TABLE MODIFY QUERY", VIEW, ALTER_VIEW) \ diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index bcd80fe62e7..6d4ac96e56a 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -34,6 +34,7 @@ namespace ErrorCodes extern const int LOGICAL_ERROR; extern const int QUERY_NOT_ALLOWED; extern const int UNKNOWN_TABLE; + extern const int BAD_ARGUMENTS; } DatabaseMaterializedPostgreSQL::DatabaseMaterializedPostgreSQL( diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index 0a40275b4bb..9cc4e02c4c4 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -48,7 +48,7 @@ BlockIO InterpreterAlterQuery::execute() else if (alter.alter_object == ASTAlterQuery::AlterObjectType::TABLE || alter.alter_object == ASTAlterQuery::AlterObjectType::LIVE_VIEW) return executeToTable(alter); - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown alter object type"); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unknown alter object type"); } diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index 8974ad93149..24fafff4123 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -166,7 +166,8 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) auto * materialized_storage = storage->as (); try { - /// TODO: THIS IS INCORRENT, we might get here if there is no nested, need to check and reload. + /// FIXME: Looks like it is possible we might get here if there is no nested storage or at least nested storage id field might be empty. + /// Caught it somehow when doing something else incorrectly, but do not see any reason how it could happen. /// Try load nested table, set materialized table metadata. nested_storages[table_name] = materialized_storage->prepare(); } @@ -617,7 +618,7 @@ PostgreSQLTableStructurePtr PostgreSQLReplicationHandler::fetchTableStructure( void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPostgreSQL * materialized_storage, const String & postgres_table_name) { /// Note: we have to ensure that replication consumer task is stopped when we reload table, because otherwise - /// it can read wal beyond start lsn position (from which this table is being loaded), which will result in loosing data. + /// it can read wal beyond start lsn position (from which this table is being loaded), which will result in losing data. consumer_task->deactivate(); try { @@ -663,7 +664,7 @@ void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPost throw Exception(ErrorCodes::POSTGRESQL_REPLICATION_INTERNAL_ERROR, "Failed to add table `{}` to replication. Info: {}", postgres_table_name, error_message); } - consumer_task->schedule(); + consumer_task->activateAndSchedule(); } diff --git a/tests/queries/0_stateless/01271_show_privileges.reference b/tests/queries/0_stateless/01271_show_privileges.reference index 46eb3bf9ba8..07f670e9afb 100644 --- a/tests/queries/0_stateless/01271_show_privileges.reference +++ b/tests/queries/0_stateless/01271_show_privileges.reference @@ -35,7 +35,9 @@ ALTER SETTINGS ['ALTER SETTING','ALTER MODIFY SETTING','MODIFY SETTING','RESET S ALTER MOVE PARTITION ['ALTER MOVE PART','MOVE PARTITION','MOVE PART'] TABLE ALTER TABLE ALTER FETCH PARTITION ['ALTER FETCH PART','FETCH PARTITION'] TABLE ALTER TABLE ALTER FREEZE PARTITION ['FREEZE PARTITION','UNFREEZE'] TABLE ALTER TABLE +ALTER DATABASE SETTINGS ['ALTER DATABASE SETTING','ALTER MODIFY DATABASE SETTING','MODIFY DATABASE SETTING'] DATABASE ALTER DATABASE ALTER TABLE [] \N ALTER +ALTER DATABASE [] \N ALTER ALTER VIEW REFRESH ['ALTER LIVE VIEW REFRESH','REFRESH VIEW'] VIEW ALTER VIEW ALTER VIEW MODIFY QUERY ['ALTER TABLE MODIFY QUERY'] VIEW ALTER VIEW ALTER VIEW [] \N ALTER From 9d0444774aa05c8c6bffb7eadaf03d1f231f652f Mon Sep 17 00:00:00 2001 From: kssenii Date: Sun, 29 Aug 2021 11:50:03 +0300 Subject: [PATCH 08/13] Fix tests --- src/Databases/DatabaseAtomic.cpp | 6 ++---- src/Databases/DatabaseAtomic.h | 2 +- src/Databases/DatabaseFactory.cpp | 2 +- src/Databases/IDatabase.h | 4 ++-- src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp | 5 +++-- src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h | 4 ++-- src/Storages/AlterCommands.cpp | 5 ++++- src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp | 2 +- 8 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index e16db6477d8..cc350e58dcf 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -568,12 +568,10 @@ void DatabaseAtomic::checkDetachedTableNotInUse(const UUID & uuid) assertDetachedTableNotInUse(uuid); } -void DatabaseAtomic::modifySettings(const SettingsChanges & settings_changes, ContextPtr local_context) +void DatabaseAtomic::modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) { - applySettings(settings_changes, local_context); - auto create_query = getCreateDatabaseQuery()->clone(); - auto create = create_query->as(); + auto * create = create_query->as(); auto * settings = create->storage->settings; if (settings) { diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index 0b00a4eb43a..b0910f29ab1 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -61,7 +61,7 @@ public: void checkDetachedTableNotInUse(const UUID & uuid) override; void setDetachedTableNotInUseForce(const UUID & uuid); - void modifySettings(const SettingsChanges & settings_changes, ContextPtr local_context) override; + void modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) override; protected: void commitAlterTable(const StorageID & table_id, const String & table_metadata_tmp_path, const String & table_metadata_path, const String & statement, ContextPtr query_context) override; diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index cff71a0e7fd..962177f6f49 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -105,7 +105,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String static const std::unordered_set database_engines{"Ordinary", "Atomic", "Memory", "Dictionary", "Lazy", "Replicated", "MySQL", "MaterializeMySQL", "MaterializedMySQL", - "Lazy", "Replicated", "PostgreSQL", "MaterializedPostgreSQL", "SQLite"}; + "PostgreSQL", "MaterializedPostgreSQL", "SQLite"}; if (!database_engines.contains(engine_name)) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine name `{}` does not exist", engine_name); diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index f3f801e620b..d4e5903ad8c 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -279,12 +279,12 @@ public: throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter is not supported by database engine {}", getEngineName()); } - virtual void modifySettings(const SettingsChanges &, ContextPtr) + virtual void modifySettingsMetadata(const SettingsChanges &, ContextPtr) { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} does not support settings", getEngineName()); } - virtual void applySettings(const SettingsChanges &, ContextPtr) + virtual void tryApplySettings(const SettingsChanges &, ContextPtr) { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} does not support settings", getEngineName()); } diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 6d4ac96e56a..f720154c1ad 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -140,7 +140,7 @@ void DatabaseMaterializedPostgreSQL::checkAlterIsPossible(const AlterCommands & } -void DatabaseMaterializedPostgreSQL::applySettings(const SettingsChanges & settings_changes, ContextPtr local_context) +void DatabaseMaterializedPostgreSQL::tryApplySettings(const SettingsChanges & settings_changes, ContextPtr local_context) { for (const auto & change : settings_changes) { @@ -208,7 +208,7 @@ void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const /// Create ReplacingMergeTree table. auto query_copy = query->clone(); - auto create_query = assert_cast(query_copy.get()); + auto * create_query = assert_cast(query_copy.get()); create_query->attach = false; create_query->attach_short_syntax = false; DatabaseAtomic::createTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, table, query_copy); @@ -273,6 +273,7 @@ ASTPtr DatabaseMaterializedPostgreSQL::createAlterSettingsQuery(const SettingCha void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, const StoragePtr & table, const String & relative_table_path) { + /// TODO: If attach fails, need to delete nested... if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) { auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index 40abb24cccf..c33b6274e7c 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -64,7 +64,7 @@ public: void checkAlterIsPossible(const AlterCommands & commands, ContextPtr context) const override; - void applySettings(const SettingsChanges & settings_changes, ContextPtr context) override; + void tryApplySettings(const SettingsChanges & settings_changes, ContextPtr context) override; void shutdown() override; @@ -76,7 +76,7 @@ protected: private: void startSynchronization(); - ASTPtr createAlterSettingsQuery(const SettingChange & change); + ASTPtr createAlterSettingsQuery(const SettingChange & new_setting); String getTablesList(const String & except = {}) const; diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index de9d9e594fd..6ee3db90724 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -365,7 +365,10 @@ void AlterCommands::apply(DatabasePtr database, ContextPtr context) const if (!command.ignore) { if (command.type == AlterCommand::MODIFY_DATABASE_SETTING) - database->modifySettings(command.settings_changes, context); + { + database->tryApplySettings(command.settings_changes, context); + database->modifySettingsMetadata(command.settings_changes, context); + } else throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unsupported alter command"); } diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp index 1bf3c5ce1ca..e38ea1e0958 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp @@ -51,7 +51,7 @@ StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL( std::unique_ptr replication_settings) : IStorage(table_id_) , WithContext(context_->getGlobalContext()) - , log(&Poco::Logger::get("StorageMaterializedPostgreSQL(" + postgres::formatNameForLogs(remote_database_name, remote_table_name) + ")")) + , log(&Poco::Logger::get("StorageMaterializedPostgreSQL(" + postgres::formatNameForLogs(remote_database_name, remote_table_name_) + ")")) , is_materialized_postgresql_database(false) , has_nested(false) , nested_context(makeNestedTableContext(context_->getGlobalContext())) From fbd2f40b051cdf030d83d9386b9afed19b366859 Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 3 Sep 2021 09:04:46 +0000 Subject: [PATCH 09/13] Fix --- src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp | 1 + src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp b/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp index 90917a0fd7e..f2860235117 100644 --- a/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp +++ b/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp @@ -571,6 +571,7 @@ ASTs InterpreterAlterImpl::getRewrittenQueries( auto rewritten_rename_query = std::make_shared(); rewritten_alter_query->database = mapped_to_database; rewritten_alter_query->table = alter_query.table; + rewritten_alter_query->alter_object = ASTAlterQuery::AlterObjectType::TABLE; rewritten_alter_query->set(rewritten_alter_query->command_list, std::make_shared()); String default_after_column; diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index c47e003a681..7cc3340bbb7 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -456,7 +456,7 @@ void PostgreSQLReplicationHandler::setSetting(const SettingChange & setting) { consumer_task->deactivate(); consumer->setSetting(setting); - consumer_task->schedule(); + consumer_task->activateAndSchedule(); } @@ -658,6 +658,7 @@ void PostgreSQLReplicationHandler::addTableToReplication(StorageMaterializedPost } catch (...) { + consumer_task->activate(); consumer_task->scheduleAfter(RESCHEDULE_MS); auto error_message = getCurrentExceptionMessage(false); @@ -685,13 +686,14 @@ void PostgreSQLReplicationHandler::removeTableFromReplication(const String & pos } catch (...) { + consumer_task->activate(); consumer_task->scheduleAfter(RESCHEDULE_MS); auto error_message = getCurrentExceptionMessage(false); throw Exception(ErrorCodes::POSTGRESQL_REPLICATION_INTERNAL_ERROR, "Failed to remove table `{}` from replication. Info: {}", postgres_table_name, error_message); } - consumer_task->schedule(); + consumer_task->activateAndSchedule(); } From 76e49334fa8073a7208cce6cd1dcb5e1d968ee30 Mon Sep 17 00:00:00 2001 From: kssenii Date: Sun, 5 Sep 2021 01:59:44 +0300 Subject: [PATCH 10/13] Minor fixes --- src/Databases/DatabaseAtomic.cpp | 53 +--------- src/Databases/DatabaseAtomic.h | 3 - src/Databases/DatabaseFactory.cpp | 4 +- src/Databases/DatabaseLazy.cpp | 4 +- src/Databases/DatabaseLazy.h | 2 +- src/Databases/DatabaseOnDisk.cpp | 53 +++++++++- src/Databases/DatabaseOnDisk.h | 6 +- src/Databases/DatabaseOrdinary.cpp | 8 +- src/Databases/DatabaseOrdinary.h | 5 +- .../MySQL/DatabaseMaterializedMySQL.cpp | 4 +- .../DatabaseMaterializedPostgreSQL.cpp | 97 +++++++++++-------- src/Parsers/ParserAlterQuery.cpp | 14 +++ .../test.py | 4 + 13 files changed, 149 insertions(+), 108 deletions(-) diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index e73a8114c00..1162eee5fef 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -38,11 +38,10 @@ public: }; DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_, ASTPtr storage_def_) - : DatabaseOrdinary(name_, std::move(metadata_path_), "store/", logger_name, context_) + : DatabaseOrdinary(name_, std::move(metadata_path_), "store/", logger_name, context_, storage_def_) , path_to_table_symlinks(fs::path(getContext()->getPath()) / "data" / escapeForFileName(name_) / "") , path_to_metadata_symlink(fs::path(getContext()->getPath()) / "metadata" / escapeForFileName(name_)) , db_uuid(uuid) - , storage_def(storage_def_) { assert(db_uuid != UUIDHelpers::Nil); fs::create_directories(path_to_table_symlinks); @@ -569,54 +568,4 @@ void DatabaseAtomic::checkDetachedTableNotInUse(const UUID & uuid) assertDetachedTableNotInUse(uuid); } -void DatabaseAtomic::modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) -{ - auto create_query = getCreateDatabaseQuery()->clone(); - auto * create = create_query->as(); - auto * settings = create->storage->settings; - if (settings) - { - auto & previous_settings = settings->changes; - for (const auto & change : settings_changes) - { - auto it = std::find_if(previous_settings.begin(), previous_settings.end(), - [&](const auto & prev){ return prev.name == change.name; }); - if (it != previous_settings.end()) - it->value = change.value; - else - previous_settings.push_back(change); - } - } - else - { - auto settings = std::make_shared(); - settings->is_standalone = false; - settings->changes = settings_changes; - create->storage->set(create->storage->settings, settings->clone()); - } - - create->attach = true; - create->if_not_exists = false; - - WriteBufferFromOwnString statement_buf; - formatAST(*create, statement_buf, false); - writeChar('\n', statement_buf); - String statement = statement_buf.str(); - - String database_name_escaped = escapeForFileName(database_name); - fs::path metadata_root_path = fs::canonical(local_context->getGlobalContext()->getPath()); - fs::path metadata_file_tmp_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql.tmp"); - fs::path metadata_file_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql"); - - WriteBufferFromFile out(metadata_file_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); - writeString(statement, out); - - out.next(); - if (getContext()->getSettingsRef().fsync_metadata) - out.sync(); - out.close(); - - fs::rename(metadata_file_tmp_path, metadata_file_path); -} - } diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index 77d2ec6a6cb..934dcbb997e 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -61,8 +61,6 @@ public: void checkDetachedTableNotInUse(const UUID & uuid) override; void setDetachedTableNotInUseForce(const UUID & uuid); - void modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) override; - protected: void commitAlterTable(const StorageID & table_id, const String & table_metadata_tmp_path, const String & table_metadata_path, const String & statement, ContextPtr query_context) override; void commitCreateTable(const ASTCreateQuery & query, const StoragePtr & table, @@ -82,7 +80,6 @@ protected: String path_to_table_symlinks; String path_to_metadata_symlink; const UUID db_uuid; - ASTPtr storage_def; }; } diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 962177f6f49..9b732f29fe5 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -128,7 +128,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String "Database engine `{}` cannot have parameters, primary_key, order_by, sample_by, settings", engine_name); if (engine_name == "Ordinary") - return std::make_shared(database_name, metadata_path, context); + return std::make_shared(database_name, metadata_path, context, engine_define->clone()); else if (engine_name == "Atomic") return std::make_shared(database_name, metadata_path, uuid, context, engine_define->clone()); else if (engine_name == "Memory") @@ -208,7 +208,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String const auto & arguments = engine->arguments->children; const auto cache_expiration_time_seconds = safeGetLiteralValue(arguments[0], "Lazy"); - return std::make_shared(database_name, metadata_path, cache_expiration_time_seconds, context); + return std::make_shared(database_name, metadata_path, cache_expiration_time_seconds, context, engine_define->clone()); } else if (engine_name == "Replicated") diff --git a/src/Databases/DatabaseLazy.cpp b/src/Databases/DatabaseLazy.cpp index 7e0e1b7aa43..d77dcc06e36 100644 --- a/src/Databases/DatabaseLazy.cpp +++ b/src/Databases/DatabaseLazy.cpp @@ -28,8 +28,8 @@ namespace ErrorCodes } -DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_) - : DatabaseOnDisk(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseLazy (" + name_ + ")", context_) +DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_, ASTPtr storage_def_) + : DatabaseOnDisk(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseLazy (" + name_ + ")", context_, storage_def_) , expiration_time(expiration_time_) { } diff --git a/src/Databases/DatabaseLazy.h b/src/Databases/DatabaseLazy.h index bc79a49b2fe..49a9ae394f7 100644 --- a/src/Databases/DatabaseLazy.h +++ b/src/Databases/DatabaseLazy.h @@ -18,7 +18,7 @@ class Context; class DatabaseLazy final : public DatabaseOnDisk { public: - DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_); + DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_, ASTPtr storage_def_); String getEngineName() const override { return "Lazy"; } diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 620e560b64c..0bb4c35ec6b 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -196,10 +196,12 @@ DatabaseOnDisk::DatabaseOnDisk( const String & metadata_path_, const String & data_path_, const String & logger, - ContextPtr local_context) + ContextPtr local_context, + ASTPtr storage_def_) : DatabaseWithOwnTablesBase(name, logger, local_context) , metadata_path(metadata_path_) , data_path(data_path_) + , storage_def(storage_def_) { fs::create_directories(local_context->getPath() + data_path); fs::create_directories(metadata_path); @@ -699,4 +701,53 @@ ASTPtr DatabaseOnDisk::getCreateQueryFromMetadata(const String & database_metada return ast; } +void DatabaseOnDisk::modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) +{ + auto create_query = getCreateDatabaseQuery()->clone(); + auto * create = create_query->as(); + auto * settings = create->storage->settings; + if (settings) + { + auto & storage_settings = settings->changes; + for (const auto & change : settings_changes) + { + auto it = std::find_if(storage_settings.begin(), storage_settings.end(), + [&](const auto & prev){ return prev.name == change.name; }); + if (it != storage_settings.end()) + it->value = change.value; + else + storage_settings.push_back(change); + } + } + else + { + auto storage_settings = std::make_shared(); + storage_settings->is_standalone = false; + storage_settings->changes = settings_changes; + create->storage->set(create->storage->settings, storage_settings->clone()); + } + + create->attach = true; + create->if_not_exists = false; + + WriteBufferFromOwnString statement_buf; + formatAST(*create, statement_buf, false); + writeChar('\n', statement_buf); + String statement = statement_buf.str(); + + String database_name_escaped = escapeForFileName(database_name); + fs::path metadata_root_path = fs::canonical(local_context->getGlobalContext()->getPath()); + fs::path metadata_file_tmp_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql.tmp"); + fs::path metadata_file_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql"); + + WriteBufferFromFile out(metadata_file_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); + writeString(statement, out); + + out.next(); + if (getContext()->getSettingsRef().fsync_metadata) + out.sync(); + out.close(); + + fs::rename(metadata_file_tmp_path, metadata_file_path); +} } diff --git a/src/Databases/DatabaseOnDisk.h b/src/Databases/DatabaseOnDisk.h index e7dda7cb36b..ffad5f215c9 100644 --- a/src/Databases/DatabaseOnDisk.h +++ b/src/Databases/DatabaseOnDisk.h @@ -32,7 +32,7 @@ void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemo class DatabaseOnDisk : public DatabaseWithOwnTablesBase { public: - DatabaseOnDisk(const String & name, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context); + DatabaseOnDisk(const String & name, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context, ASTPtr storage_def_); void createTable( ContextPtr context, @@ -74,6 +74,8 @@ public: void checkMetadataFilenameAvailability(const String & to_table_name) const; void checkMetadataFilenameAvailabilityUnlocked(const String & to_table_name, std::unique_lock &) const; + void modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) override; + protected: static constexpr const char * create_suffix = ".tmp"; static constexpr const char * drop_suffix = ".tmp_drop"; @@ -97,6 +99,8 @@ protected: const String metadata_path; const String data_path; + + ASTPtr storage_def; }; } diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index bfe5de4c95f..991dda44f0b 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -72,14 +72,14 @@ namespace } -DatabaseOrdinary::DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context_) - : DatabaseOrdinary(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseOrdinary (" + name_ + ")", context_) +DatabaseOrdinary::DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context_, ASTPtr storage_def_) + : DatabaseOrdinary(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseOrdinary (" + name_ + ")", context_, storage_def_) { } DatabaseOrdinary::DatabaseOrdinary( - const String & name_, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context_) - : DatabaseOnDisk(name_, metadata_path_, data_path_, logger, context_) + const String & name_, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context_, ASTPtr storage_def_) + : DatabaseOnDisk(name_, metadata_path_, data_path_, logger, context_, storage_def_) { } diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index 7832377ccae..e9cbb6f22e6 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -14,9 +14,10 @@ namespace DB class DatabaseOrdinary : public DatabaseOnDisk { public: - DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context); + DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context, ASTPtr storage_def_); DatabaseOrdinary( - const String & name_, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context_); + const String & name_, const String & metadata_path_, const String & data_path_, + const String & logger, ContextPtr context_, ASTPtr storage_def_); String getEngineName() const override { return "Ordinary"; } diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 1bdf4b962ec..9bc6ae679f6 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -38,13 +38,13 @@ DatabaseMaterializedMySQL::DatabaseMaterializedMySQL( mysqlxx::Pool && pool_, MySQLClient && client_, std::unique_ptr settings_, - ASTPtr) + ASTPtr storage_def_) : DatabaseOrdinary( database_name_, metadata_path_, "data/" + escapeForFileName(database_name_) + "/", "DatabaseMaterializedMySQL (" + database_name_ + ")", - context_) + context_, storage_def_) , settings(std::move(settings_)) , materialize_thread(context_, database_name_, mysql_database_name_, std::move(pool_), std::move(client_), settings.get()) { diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 5c9cc95238b..4d5e4ff2d7b 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -194,31 +194,6 @@ StoragePtr DatabaseMaterializedPostgreSQL::tryGetTable(const String & name, Cont } -void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const String & table_name, const StoragePtr & table, const ASTPtr & query) -{ - /// Create table query can only be called from replication thread. - if (local_context->isInternalQuery()) - { - DatabaseAtomic::createTable(local_context, table_name, table, query); - return; - } - - const auto & create = query->as(); - if (!create->attach) - throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "CREATE TABLE is not allowed for database engine {}. Use ATTACH TABLE instead", getEngineName()); - - /// Create ReplacingMergeTree table. - auto query_copy = query->clone(); - auto * create_query = assert_cast(query_copy.get()); - create_query->attach = false; - create_query->attach_short_syntax = false; - DatabaseAtomic::createTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, table, query_copy); - - /// Attach MaterializedPostgreSQL table. - attachTable(table_name, table, {}); -} - - String DatabaseMaterializedPostgreSQL::getTablesList(const String & except) const { String tables_list; @@ -272,26 +247,64 @@ ASTPtr DatabaseMaterializedPostgreSQL::createAlterSettingsQuery(const SettingCha } +void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const String & table_name, const StoragePtr & table, const ASTPtr & query) +{ + /// Create table query can only be called from replication thread. + if (local_context->isInternalQuery()) + { + DatabaseAtomic::createTable(local_context, table_name, table, query); + return; + } + + const auto & create = query->as(); + if (!create->attach) + throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "CREATE TABLE is not allowed for database engine {}. Use ATTACH TABLE instead", getEngineName()); + + /// Create ReplacingMergeTree table. + auto query_copy = query->clone(); + auto * create_query = assert_cast(query_copy.get()); + create_query->attach = false; + create_query->attach_short_syntax = false; + DatabaseAtomic::createTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, table, query_copy); + + /// Attach MaterializedPostgreSQL table. + attachTable(table_name, table, {}); +} + + void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, const StoragePtr & table, const String & relative_table_path) { - /// TODO: If attach fails, need to delete nested... if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) { - auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; - if (tables_to_replicate.empty()) - tables_to_replicate = getTablesList(); - - /// tables_to_replicate can be empty if postgres database had no tables when this database was created. - SettingChange new_setting("materialized_postgresql_tables_list", tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name)); - auto alter_query = createAlterSettingsQuery(new_setting); - auto current_context = Context::createCopy(getContext()->getGlobalContext()); current_context->setInternalQuery(true); - InterpreterAlterQuery(alter_query, current_context).execute(); - auto storage = StorageMaterializedPostgreSQL::create(table, getContext(), remote_database_name, table_name); - materialized_tables[table_name] = storage; - replication_handler->addTableToReplication(dynamic_cast(storage.get()), table_name); + /// We just came from createTable() and created nested table there. Add assert. + auto nested_table = DatabaseAtomic::tryGetTable(table_name, current_context); + assert(nested_table != nullptr); + + try + { + auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; + if (tables_to_replicate.empty()) + tables_to_replicate = getTablesList(); + + /// tables_to_replicate can be empty if postgres database had no tables when this database was created. + SettingChange new_setting("materialized_postgresql_tables_list", tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name)); + auto alter_query = createAlterSettingsQuery(new_setting); + + InterpreterAlterQuery(alter_query, current_context).execute(); + + auto storage = StorageMaterializedPostgreSQL::create(table, getContext(), remote_database_name, table_name); + materialized_tables[table_name] = storage; + replication_handler->addTableToReplication(dynamic_cast(storage.get()), table_name); + } + catch (...) + { + /// This is a failed attach table. Remove already created nested table. + DatabaseAtomic::dropTable(current_context, table_name, true); + throw; + } } else { @@ -334,6 +347,14 @@ StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name } catch (Exception & e) { + /// We already removed this table from replication and adding it back will be an overkill.. + /// TODO: this is bad, we leave a table lying somewhere not dropped, and if user will want + /// to move it back into replication, he will fail to do so because there is undropped nested with the same name. + /// This can also happen if we crash after removing table from replication andd before dropping nested. + /// As a solution, we could drop a table if it already exists and add a fresh one instead for these two cases. + /// TODO: sounds good. + materialized_tables.erase(table_name); + e.addMessage("while removing table `" + table_name + "` from replication"); throw; } diff --git a/src/Parsers/ParserAlterQuery.cpp b/src/Parsers/ParserAlterQuery.cpp index bd998e962d1..e89302fd212 100644 --- a/src/Parsers/ParserAlterQuery.cpp +++ b/src/Parsers/ParserAlterQuery.cpp @@ -187,6 +187,20 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected command->type = ASTAlterCommand::RENAME_COLUMN; } + else if (s_materialize_column.ignore(pos, expected)) + { + if (!parser_name.parse(pos, command->column, expected)) + return false; + + command->type = ASTAlterCommand::MATERIALIZE_COLUMN; + command->detach = false; + + if (s_in_partition.ignore(pos, expected)) + { + if (!parser_partition.parse(pos, command->partition, expected)) + return false; + } + } else if (s_drop_partition.ignore(pos, expected)) { if (!parser_partition.parse(pos, command->partition, expected)) diff --git a/tests/integration/test_postgresql_replica_database_engine/test.py b/tests/integration/test_postgresql_replica_database_engine/test.py index 4c12966bef7..0c51bb3aafb 100644 --- a/tests/integration/test_postgresql_replica_database_engine/test.py +++ b/tests/integration/test_postgresql_replica_database_engine/test.py @@ -928,6 +928,10 @@ def test_abrupt_server_restart_while_heavy_replication(started_cluster): def test_quoting(started_cluster): table_name = 'user' + conn = get_postgres_conn(ip=started_cluster.postgres_ip, + port=started_cluster.postgres_port, + database=True) + cursor = conn.cursor() create_postgres_table(cursor, table_name); instance.query("INSERT INTO postgres_database.{} SELECT number, number from numbers(50)".format(table_name)) create_materialized_db(ip=started_cluster.postgres_ip, port=started_cluster.postgres_port) From bed2688dad02b83d77240b930b6c85775c24691a Mon Sep 17 00:00:00 2001 From: kssenii Date: Thu, 9 Sep 2021 01:25:08 +0300 Subject: [PATCH 11/13] Review fixes --- src/Databases/DatabaseAtomic.cpp | 8 ++-- src/Databases/DatabaseAtomic.h | 4 +- src/Databases/DatabaseFactory.cpp | 18 ++++---- src/Databases/DatabaseLazy.cpp | 4 +- src/Databases/DatabaseLazy.h | 2 +- src/Databases/DatabaseOnDisk.cpp | 10 ++--- src/Databases/DatabaseOnDisk.h | 7 ++-- src/Databases/DatabaseOrdinary.cpp | 8 ++-- src/Databases/DatabaseOrdinary.h | 4 +- src/Databases/DatabaseReplicated.cpp | 5 +-- src/Databases/DatabaseReplicated.h | 2 +- src/Databases/IDatabase.h | 16 ++------ .../MySQL/DatabaseMaterializedMySQL.cpp | 10 ++--- .../MySQL/DatabaseMaterializedMySQL.h | 2 +- .../DatabaseMaterializedPostgreSQL.cpp | 41 +++++++++++-------- .../DatabaseMaterializedPostgreSQL.h | 8 ++-- src/Interpreters/InterpreterAlterQuery.cpp | 19 ++++++++- src/Storages/AlterCommands.cpp | 18 -------- src/Storages/AlterCommands.h | 2 +- .../MaterializedPostgreSQLConsumer.cpp | 2 +- .../PostgreSQLReplicationHandler.cpp | 6 +-- .../StorageMaterializedPostgreSQL.cpp | 3 +- .../test.py | 4 ++ 23 files changed, 96 insertions(+), 107 deletions(-) diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 1162eee5fef..7b1a8c6446e 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -37,8 +37,8 @@ public: UUID uuid() const override { return table()->getStorageID().uuid; } }; -DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_, ASTPtr storage_def_) - : DatabaseOrdinary(name_, std::move(metadata_path_), "store/", logger_name, context_, storage_def_) +DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_) + : DatabaseOrdinary(name_, std::move(metadata_path_), "store/", logger_name, context_) , path_to_table_symlinks(fs::path(getContext()->getPath()) / "data" / escapeForFileName(name_) / "") , path_to_metadata_symlink(fs::path(getContext()->getPath()) / "metadata" / escapeForFileName(name_)) , db_uuid(uuid) @@ -48,8 +48,8 @@ DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, c tryCreateMetadataSymlink(); } -DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_, ASTPtr storage_def_) - : DatabaseAtomic(name_, std::move(metadata_path_), uuid, "DatabaseAtomic (" + name_ + ")", context_, storage_def_) +DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_) + : DatabaseAtomic(name_, std::move(metadata_path_), uuid, "DatabaseAtomic (" + name_ + ")", context_) { } diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index 934dcbb997e..8be009cd6ca 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -19,8 +19,8 @@ namespace DB class DatabaseAtomic : public DatabaseOrdinary { public: - DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_, ASTPtr storage_def_); - DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_, ASTPtr storage_def_); + DatabaseAtomic(String name_, String metadata_path_, UUID uuid, const String & logger_name, ContextPtr context_); + DatabaseAtomic(String name_, String metadata_path_, UUID uuid, ContextPtr context_); String getEngineName() const override { return "Atomic"; } UUID getUUID() const override { return db_uuid; } diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 9b732f29fe5..047d4a55802 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -128,9 +128,9 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String "Database engine `{}` cannot have parameters, primary_key, order_by, sample_by, settings", engine_name); if (engine_name == "Ordinary") - return std::make_shared(database_name, metadata_path, context, engine_define->clone()); + return std::make_shared(database_name, metadata_path, context); else if (engine_name == "Atomic") - return std::make_shared(database_name, metadata_path, uuid, context, engine_define->clone()); + return std::make_shared(database_name, metadata_path, uuid, context); else if (engine_name == "Memory") return std::make_shared(database_name, context); else if (engine_name == "Dictionary") @@ -183,12 +183,12 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String if (create.uuid == UUIDHelpers::Nil) return std::make_shared>( - context, database_name, metadata_path, uuid, mysql_database_name, std::move(mysql_pool), std::move(client) - , std::move(materialize_mode_settings), engine_define->clone()); + context, database_name, metadata_path, uuid, mysql_database_name, + std::move(mysql_pool), std::move(client), std::move(materialize_mode_settings)); else return std::make_shared>( - context, database_name, metadata_path, uuid, mysql_database_name, std::move(mysql_pool), std::move(client) - , std::move(materialize_mode_settings), engine_define->clone()); + context, database_name, metadata_path, uuid, mysql_database_name, + std::move(mysql_pool), std::move(client), std::move(materialize_mode_settings)); } catch (...) { @@ -208,7 +208,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String const auto & arguments = engine->arguments->children; const auto cache_expiration_time_seconds = safeGetLiteralValue(arguments[0], "Lazy"); - return std::make_shared(database_name, metadata_path, cache_expiration_time_seconds, context, engine_define->clone()); + return std::make_shared(database_name, metadata_path, cache_expiration_time_seconds, context); } else if (engine_name == "Replicated") @@ -234,7 +234,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String return std::make_shared(database_name, metadata_path, uuid, zookeeper_path, shard_name, replica_name, - std::move(database_replicated_settings), context, engine_define->clone()); + std::move(database_replicated_settings), context); } #if USE_LIBPQXX @@ -311,7 +311,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String postgresql_replica_settings->loadFromQuery(*engine_define); return std::make_shared( - context, metadata_path, uuid, engine_define->clone(), create.attach, + context, metadata_path, uuid, create.attach, database_name, postgres_database_name, connection_info, std::move(postgresql_replica_settings)); } diff --git a/src/Databases/DatabaseLazy.cpp b/src/Databases/DatabaseLazy.cpp index d77dcc06e36..7e0e1b7aa43 100644 --- a/src/Databases/DatabaseLazy.cpp +++ b/src/Databases/DatabaseLazy.cpp @@ -28,8 +28,8 @@ namespace ErrorCodes } -DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_, ASTPtr storage_def_) - : DatabaseOnDisk(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseLazy (" + name_ + ")", context_, storage_def_) +DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_) + : DatabaseOnDisk(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseLazy (" + name_ + ")", context_) , expiration_time(expiration_time_) { } diff --git a/src/Databases/DatabaseLazy.h b/src/Databases/DatabaseLazy.h index 49a9ae394f7..bc79a49b2fe 100644 --- a/src/Databases/DatabaseLazy.h +++ b/src/Databases/DatabaseLazy.h @@ -18,7 +18,7 @@ class Context; class DatabaseLazy final : public DatabaseOnDisk { public: - DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_, ASTPtr storage_def_); + DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, ContextPtr context_); String getEngineName() const override { return "Lazy"; } diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 0bb4c35ec6b..f5b930a83c7 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -196,12 +196,10 @@ DatabaseOnDisk::DatabaseOnDisk( const String & metadata_path_, const String & data_path_, const String & logger, - ContextPtr local_context, - ASTPtr storage_def_) + ContextPtr local_context) : DatabaseWithOwnTablesBase(name, logger, local_context) , metadata_path(metadata_path_) , data_path(data_path_) - , storage_def(storage_def_) { fs::create_directories(local_context->getPath() + data_path); fs::create_directories(metadata_path); @@ -701,8 +699,10 @@ ASTPtr DatabaseOnDisk::getCreateQueryFromMetadata(const String & database_metada return ast; } -void DatabaseOnDisk::modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) +void DatabaseOnDisk::modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr query_context) { + std::lock_guard lock(modify_settings_mutex); + auto create_query = getCreateDatabaseQuery()->clone(); auto * create = create_query->as(); auto * settings = create->storage->settings; @@ -736,7 +736,7 @@ void DatabaseOnDisk::modifySettingsMetadata(const SettingsChanges & settings_cha String statement = statement_buf.str(); String database_name_escaped = escapeForFileName(database_name); - fs::path metadata_root_path = fs::canonical(local_context->getGlobalContext()->getPath()); + fs::path metadata_root_path = fs::canonical(query_context->getGlobalContext()->getPath()); fs::path metadata_file_tmp_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql.tmp"); fs::path metadata_file_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql"); diff --git a/src/Databases/DatabaseOnDisk.h b/src/Databases/DatabaseOnDisk.h index ffad5f215c9..e375704be33 100644 --- a/src/Databases/DatabaseOnDisk.h +++ b/src/Databases/DatabaseOnDisk.h @@ -32,7 +32,7 @@ void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemo class DatabaseOnDisk : public DatabaseWithOwnTablesBase { public: - DatabaseOnDisk(const String & name, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context, ASTPtr storage_def_); + DatabaseOnDisk(const String & name, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context); void createTable( ContextPtr context, @@ -74,7 +74,7 @@ public: void checkMetadataFilenameAvailability(const String & to_table_name) const; void checkMetadataFilenameAvailabilityUnlocked(const String & to_table_name, std::unique_lock &) const; - void modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr local_context) override; + void modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr query_context); protected: static constexpr const char * create_suffix = ".tmp"; @@ -100,7 +100,8 @@ protected: const String metadata_path; const String data_path; - ASTPtr storage_def; + /// For alter settings. + std::mutex modify_settings_mutex; }; } diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 991dda44f0b..bfe5de4c95f 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -72,14 +72,14 @@ namespace } -DatabaseOrdinary::DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context_, ASTPtr storage_def_) - : DatabaseOrdinary(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseOrdinary (" + name_ + ")", context_, storage_def_) +DatabaseOrdinary::DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context_) + : DatabaseOrdinary(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseOrdinary (" + name_ + ")", context_) { } DatabaseOrdinary::DatabaseOrdinary( - const String & name_, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context_, ASTPtr storage_def_) - : DatabaseOnDisk(name_, metadata_path_, data_path_, logger, context_, storage_def_) + const String & name_, const String & metadata_path_, const String & data_path_, const String & logger, ContextPtr context_) + : DatabaseOnDisk(name_, metadata_path_, data_path_, logger, context_) { } diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index e9cbb6f22e6..5540632d60c 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -14,10 +14,10 @@ namespace DB class DatabaseOrdinary : public DatabaseOnDisk { public: - DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context, ASTPtr storage_def_); + DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context); DatabaseOrdinary( const String & name_, const String & metadata_path_, const String & data_path_, - const String & logger, ContextPtr context_, ASTPtr storage_def_); + const String & logger, ContextPtr context_); String getEngineName() const override { return "Ordinary"; } diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 28481267a33..da03eb6aba6 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -67,9 +67,8 @@ DatabaseReplicated::DatabaseReplicated( const String & shard_name_, const String & replica_name_, DatabaseReplicatedSettings db_settings_, - ContextPtr context_, - ASTPtr storage_def_) - : DatabaseAtomic(name_, metadata_path_, uuid, "DatabaseReplicated (" + name_ + ")", context_, storage_def_) + ContextPtr context_) + : DatabaseAtomic(name_, metadata_path_, uuid, "DatabaseReplicated (" + name_ + ")", context_) , zookeeper_path(zookeeper_path_) , shard_name(shard_name_) , replica_name(replica_name_) diff --git a/src/Databases/DatabaseReplicated.h b/src/Databases/DatabaseReplicated.h index b75a29f6333..1e0daeed07e 100644 --- a/src/Databases/DatabaseReplicated.h +++ b/src/Databases/DatabaseReplicated.h @@ -24,7 +24,7 @@ public: DatabaseReplicated(const String & name_, const String & metadata_path_, UUID uuid, const String & zookeeper_path_, const String & shard_name_, const String & replica_name_, DatabaseReplicatedSettings db_settings_, - ContextPtr context, ASTPtr storage_def_); + ContextPtr context); ~DatabaseReplicated() override; diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index 3e181b8e3a2..92041b366a7 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -282,19 +282,11 @@ public: /// Delete data and metadata stored inside the database, if exists. virtual void drop(ContextPtr /*context*/) {} - virtual void checkAlterIsPossible(const AlterCommands & /* commands */, ContextPtr /* context */) const + virtual void applyNewSettings(const SettingsChanges &, ContextPtr) { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter is not supported by database engine {}", getEngineName()); - } - - virtual void modifySettingsMetadata(const SettingsChanges &, ContextPtr) - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} does not support settings", getEngineName()); - } - - virtual void tryApplySettings(const SettingsChanges &, ContextPtr) - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} does not support settings", getEngineName()); + throw Exception(ErrorCodes::NOT_IMPLEMENTED, + "Database engine {} either does not support settings, or does not support altering settings", + getEngineName()); } virtual ~IDatabase() = default; diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 9bc6ae679f6..0d81a4e1a98 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -37,14 +37,13 @@ DatabaseMaterializedMySQL::DatabaseMaterializedMySQL( const String & mysql_database_name_, mysqlxx::Pool && pool_, MySQLClient && client_, - std::unique_ptr settings_, - ASTPtr storage_def_) + std::unique_ptr settings_) : DatabaseOrdinary( database_name_, metadata_path_, "data/" + escapeForFileName(database_name_) + "/", "DatabaseMaterializedMySQL (" + database_name_ + ")", - context_, storage_def_) + context_) , settings(std::move(settings_)) , materialize_thread(context_, database_name_, mysql_database_name_, std::move(pool_), std::move(client_), settings.get()) { @@ -59,9 +58,8 @@ DatabaseMaterializedMySQL::DatabaseMaterializedMySQL( const String & mysql_database_name_, mysqlxx::Pool && pool_, MySQLClient && client_, - std::unique_ptr settings_, - ASTPtr storage_def_) - : DatabaseAtomic(database_name_, metadata_path_, uuid, "DatabaseMaterializedMySQL (" + database_name_ + ")", context_, storage_def_) + std::unique_ptr settings_) + : DatabaseAtomic(database_name_, metadata_path_, uuid, "DatabaseMaterializedMySQL (" + database_name_ + ")", context_) , settings(std::move(settings_)) , materialize_thread(context_, database_name_, mysql_database_name_, std::move(pool_), std::move(client_), settings.get()) { diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.h b/src/Databases/MySQL/DatabaseMaterializedMySQL.h index 0339df5a156..292edc97878 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.h +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.h @@ -25,7 +25,7 @@ public: DatabaseMaterializedMySQL( ContextPtr context, const String & database_name_, const String & metadata_path_, UUID uuid, const String & mysql_database_name_, mysqlxx::Pool && pool_, - MySQLClient && client_, std::unique_ptr settings_, ASTPtr storage_def_); + MySQLClient && client_, std::unique_ptr settings_); void rethrowExceptionIfNeed() const; diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index e00f067e1bc..0e441efc428 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -41,13 +41,12 @@ DatabaseMaterializedPostgreSQL::DatabaseMaterializedPostgreSQL( ContextPtr context_, const String & metadata_path_, UUID uuid_, - ASTPtr storage_def_, bool is_attach_, const String & database_name_, const String & postgres_database_name, const postgres::ConnectionInfo & connection_info_, std::unique_ptr settings_) - : DatabaseAtomic(database_name_, metadata_path_, uuid_, "DatabaseMaterializedPostgreSQL (" + database_name_ + ")", context_, storage_def_) + : DatabaseAtomic(database_name_, metadata_path_, uuid_, "DatabaseMaterializedPostgreSQL (" + database_name_ + ")", context_) , is_attach(is_attach_) , remote_database_name(postgres_database_name) , connection_info(connection_info_) @@ -129,18 +128,9 @@ void DatabaseMaterializedPostgreSQL::loadStoredObjects( } -void DatabaseMaterializedPostgreSQL::checkAlterIsPossible(const AlterCommands & commands, ContextPtr) const -{ - for (const auto & command : commands) - { - if (command.type != AlterCommand::MODIFY_DATABASE_SETTING) - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter of type '{}' is not supported by database engine {}", alterTypeToString(command.type), getEngineName()); - } -} - - -void DatabaseMaterializedPostgreSQL::tryApplySettings(const SettingsChanges & settings_changes, ContextPtr local_context) +void DatabaseMaterializedPostgreSQL::applyNewSettings(const SettingsChanges & settings_changes, ContextPtr query_context) { + std::lock_guard lock(handler_mutex); for (const auto & change : settings_changes) { if (!settings->has(change.name)) @@ -148,11 +138,14 @@ void DatabaseMaterializedPostgreSQL::tryApplySettings(const SettingsChanges & se if ((change.name == "materialized_postgresql_tables_list")) { - if (!local_context->isInternalQuery()) + if (!query_context->isInternalQuery()) throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "Changing setting `{}` is not allowed", change.name); + + DatabaseOnDisk::modifySettingsMetadata(settings_changes, query_context); } else if ((change.name == "materialized_postgresql_allow_automatic_update") || (change.name == "materialized_postgresql_max_block_size")) { + DatabaseOnDisk::modifySettingsMetadata(settings_changes, query_context); replication_handler->setSetting(change); } else @@ -192,7 +185,8 @@ StoragePtr DatabaseMaterializedPostgreSQL::tryGetTable(const String & name, Cont } -String DatabaseMaterializedPostgreSQL::getTablesList(const String & except) const +/// `except` is not empty in case it is detach and it will contain only one table name - name of detached table. +String DatabaseMaterializedPostgreSQL::getFormattedTablesList(const String & except) const { String tables_list; for (const auto & table : materialized_tables) @@ -214,6 +208,8 @@ ASTPtr DatabaseMaterializedPostgreSQL::getCreateTableQueryImpl(const String & ta if (!local_context->hasQueryContext()) return DatabaseAtomic::getCreateTableQueryImpl(table_name, local_context, throw_on_error); + std::lock_guard lock(handler_mutex); + auto storage = StorageMaterializedPostgreSQL::create(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); auto ast_storage = replication_handler->getCreateNestedTableQuery(storage.get(), table_name); assert_cast(ast_storage.get())->uuid = UUIDHelpers::generateV4(); @@ -272,6 +268,8 @@ void DatabaseMaterializedPostgreSQL::createTable(ContextPtr local_context, const void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, const StoragePtr & table, const String & relative_table_path) { + /// If there is query context then we need to attach materialized storage. + /// If there is no query context then we need to attach internal storage from atomic database. if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) { auto current_context = Context::createCopy(getContext()->getGlobalContext()); @@ -285,7 +283,7 @@ void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, cons { auto tables_to_replicate = settings->materialized_postgresql_tables_list.value; if (tables_to_replicate.empty()) - tables_to_replicate = getTablesList(); + tables_to_replicate = getFormattedTablesList(); /// tables_to_replicate can be empty if postgres database had no tables when this database was created. SettingChange new_setting("materialized_postgresql_tables_list", tables_to_replicate.empty() ? table_name : (tables_to_replicate + "," + table_name)); @@ -295,6 +293,8 @@ void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, cons auto storage = StorageMaterializedPostgreSQL::create(table, getContext(), remote_database_name, table_name); materialized_tables[table_name] = storage; + + std::lock_guard lock(handler_mutex); replication_handler->addTableToReplication(dynamic_cast(storage.get()), table_name); } catch (...) @@ -313,13 +313,15 @@ void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, cons StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name) { + /// If there is query context then we need to dettach materialized storage. + /// If there is no query context then we need to dettach internal storage from atomic database. if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) { auto & table_to_delete = materialized_tables[table_name]; if (!table_to_delete) throw Exception(ErrorCodes::UNKNOWN_TABLE, "Materialized table `{}` does not exist", table_name); - auto tables_to_replicate = getTablesList(table_name); + auto tables_to_replicate = getFormattedTablesList(table_name); /// tables_to_replicate can be empty if postgres database had no tables when this database was created. SettingChange new_setting("materialized_postgresql_tables_list", tables_to_replicate); @@ -335,6 +337,7 @@ StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name if (!nested) throw Exception(ErrorCodes::UNKNOWN_TABLE, "Inner table `{}` does not exist", table_name); + std::lock_guard lock(handler_mutex); replication_handler->removeTableFromReplication(table_name); try @@ -348,7 +351,7 @@ StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name /// We already removed this table from replication and adding it back will be an overkill.. /// TODO: this is bad, we leave a table lying somewhere not dropped, and if user will want /// to move it back into replication, he will fail to do so because there is undropped nested with the same name. - /// This can also happen if we crash after removing table from replication andd before dropping nested. + /// This can also happen if we crash after removing table from replication and before dropping nested. /// As a solution, we could drop a table if it already exists and add a fresh one instead for these two cases. /// TODO: sounds good. materialized_tables.erase(table_name); @@ -376,6 +379,7 @@ void DatabaseMaterializedPostgreSQL::shutdown() void DatabaseMaterializedPostgreSQL::stopReplication() { + std::lock_guard lock(handler_mutex); if (replication_handler) replication_handler->shutdown(); @@ -393,6 +397,7 @@ void DatabaseMaterializedPostgreSQL::dropTable(ContextPtr local_context, const S void DatabaseMaterializedPostgreSQL::drop(ContextPtr local_context) { + std::lock_guard lock(handler_mutex); if (replication_handler) replication_handler->shutdownFinal(); diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index 2d27677f524..effd0ec653a 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -32,7 +32,6 @@ public: ContextPtr context_, const String & metadata_path_, UUID uuid_, - ASTPtr storage_def_, bool is_attach_, const String & database_name_, const String & postgres_database_name, @@ -62,9 +61,7 @@ public: void stopReplication(); - void checkAlterIsPossible(const AlterCommands & commands, ContextPtr context) const override; - - void tryApplySettings(const SettingsChanges & settings_changes, ContextPtr context) override; + void applyNewSettings(const SettingsChanges & settings_changes, ContextPtr query_context) override; void shutdown() override; @@ -78,7 +75,7 @@ private: ASTPtr createAlterSettingsQuery(const SettingChange & new_setting); - String getTablesList(const String & except = {}) const; + String getFormattedTablesList(const String & except = {}) const; bool is_attach; String remote_database_name; @@ -88,6 +85,7 @@ private: std::shared_ptr replication_handler; std::map materialized_tables; mutable std::mutex tables_mutex; + mutable std::mutex handler_mutex; }; } diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index 5d93875ad77..8a23acdefc4 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -185,8 +185,23 @@ BlockIO InterpreterAlterQuery::executeToDatabase(const ASTAlterQuery & alter) if (!alter_commands.empty()) { - database->checkAlterIsPossible(alter_commands, getContext()); - alter_commands.apply(database, getContext()); + /// Only ALTER SETTING is supported. + for (const auto & command : alter_commands) + { + if (command.type != AlterCommand::MODIFY_DATABASE_SETTING) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter of type '{}' is not supported by databases", alterTypeToString(command.type)); + } + + for (const auto & command : alter_commands) + { + if (!command.ignore) + { + if (command.type == AlterCommand::MODIFY_DATABASE_SETTING) + database->applyNewSettings(command.settings_changes, getContext()); + else + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unsupported alter command"); + } + } } return res; diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 6ee3db90724..e6d60a23863 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -358,24 +358,6 @@ std::optional AlterCommand::parse(const ASTAlterCommand * command_ } -void AlterCommands::apply(DatabasePtr database, ContextPtr context) const -{ - for (const AlterCommand & command : *this) - { - if (!command.ignore) - { - if (command.type == AlterCommand::MODIFY_DATABASE_SETTING) - { - database->tryApplySettings(command.settings_changes, context); - database->modifySettingsMetadata(command.settings_changes, context); - } - else - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unsupported alter command"); - } - } -} - - void AlterCommand::apply(StorageInMemoryMetadata & metadata, ContextPtr context) const { if (type == ADD_COLUMN) diff --git a/src/Storages/AlterCommands.h b/src/Storages/AlterCommands.h index a2a1a3b6709..ae1db10fb47 100644 --- a/src/Storages/AlterCommands.h +++ b/src/Storages/AlterCommands.h @@ -197,7 +197,7 @@ public: /// Commands have to be prepared before apply. void apply(StorageInMemoryMetadata & metadata, ContextPtr context) const; - void apply(DatabasePtr database, ContextPtr context) const; + /// At least one command modify settings. bool hasSettingsAlterCommand() const; diff --git a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp index c992699a206..46033efc12e 100644 --- a/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp +++ b/src/Storages/PostgreSQL/MaterializedPostgreSQLConsumer.cpp @@ -29,7 +29,7 @@ MaterializedPostgreSQLConsumer::MaterializedPostgreSQLConsumer( bool allow_automatic_update_, Storages storages_, const String & name_for_logger) - : log(&Poco::Logger::get("PostgreSQLReplicaConsumer("+ name_for_logger +")")) + : log(&Poco::Logger::get("PostgreSQLReplicaConsumer(" + name_for_logger + ")")) , context(context_) , replication_slot_name(replication_slot_name_) , publication_name(publication_name_) diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index e763815626e..23ee7532b5e 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -500,14 +500,10 @@ void PostgreSQLReplicationHandler::shutdownFinal() dropReplicationSlot(tx, /* temporary */true); }); -<<<<<<< HEAD - connection.execWithRetry([&](pqxx::nontransaction & tx) -======= if (user_managed_slot) return; - connection->execWithRetry([&](pqxx::nontransaction & tx) ->>>>>>> 8588e4d9bb809f7ca29426934855567646d2bd01 + connection.execWithRetry([&](pqxx::nontransaction & tx) { if (isReplicationSlotExist(tx, last_committed_lsn, /* temporary */false)) dropReplicationSlot(tx, /* temporary */false); diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp index cc5b857fc16..fdded7283c4 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp @@ -75,8 +75,7 @@ StorageMaterializedPostgreSQL::StorageMaterializedPostgreSQL( getContext(), is_attach, *replication_settings, - /* is_materialized_postgresql_database */false, - remote_table_name); + /* is_materialized_postgresql_database */false); if (!is_attach) { diff --git a/tests/integration/test_postgresql_replica_database_engine/test.py b/tests/integration/test_postgresql_replica_database_engine/test.py index cef502be0fe..aa9be8e0244 100644 --- a/tests/integration/test_postgresql_replica_database_engine/test.py +++ b/tests/integration/test_postgresql_replica_database_engine/test.py @@ -985,6 +985,10 @@ def test_user_managed_slots(started_cluster): def test_add_new_table_to_replication(started_cluster): drop_materialized_db() + conn = get_postgres_conn(ip=started_cluster.postgres_ip, + port=started_cluster.postgres_port, + database=True) + cursor = conn.cursor() NUM_TABLES = 5 for i in range(NUM_TABLES): From f26da04cdfe53fb5238baaab2a54bf5698e6936c Mon Sep 17 00:00:00 2001 From: kssenii Date: Thu, 9 Sep 2021 09:53:01 +0300 Subject: [PATCH 12/13] Fix checks --- src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp | 5 ++--- src/Interpreters/InterpreterAlterQuery.cpp | 2 +- src/Storages/AlterCommands.h | 2 -- tests/integration/test_grant_and_revoke/test.py | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 0e441efc428..42720fa4eb1 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -30,7 +30,6 @@ namespace DB namespace ErrorCodes { - extern const int NOT_IMPLEMENTED; extern const int LOGICAL_ERROR; extern const int QUERY_NOT_ALLOWED; extern const int UNKNOWN_TABLE; @@ -313,8 +312,8 @@ void DatabaseMaterializedPostgreSQL::attachTable(const String & table_name, cons StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name) { - /// If there is query context then we need to dettach materialized storage. - /// If there is no query context then we need to dettach internal storage from atomic database. + /// If there is query context then we need to detach materialized storage. + /// If there is no query context then we need to detach internal storage from atomic database. if (CurrentThread::isInitialized() && CurrentThread::get().getQueryContext()) { auto & table_to_delete = materialized_tables[table_name]; diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index 8a23acdefc4..fc519124a47 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -199,7 +199,7 @@ BlockIO InterpreterAlterQuery::executeToDatabase(const ASTAlterQuery & alter) if (command.type == AlterCommand::MODIFY_DATABASE_SETTING) database->applyNewSettings(command.settings_changes, getContext()); else - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unsupported alter command"); + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Unsupported alter command"); } } } diff --git a/src/Storages/AlterCommands.h b/src/Storages/AlterCommands.h index ae1db10fb47..ad2647c321f 100644 --- a/src/Storages/AlterCommands.h +++ b/src/Storages/AlterCommands.h @@ -197,8 +197,6 @@ public: /// Commands have to be prepared before apply. void apply(StorageInMemoryMetadata & metadata, ContextPtr context) const; - - /// At least one command modify settings. bool hasSettingsAlterCommand() const; diff --git a/tests/integration/test_grant_and_revoke/test.py b/tests/integration/test_grant_and_revoke/test.py index 79fe4bf9f41..b905e4df219 100644 --- a/tests/integration/test_grant_and_revoke/test.py +++ b/tests/integration/test_grant_and_revoke/test.py @@ -151,7 +151,7 @@ def test_grant_all_on_table(): instance.query("GRANT ALL ON test.table TO A WITH GRANT OPTION") instance.query("GRANT ALL ON test.table TO B", user='A') assert instance.query( - "SHOW GRANTS FOR B") == "GRANT SHOW TABLES, SHOW COLUMNS, SHOW DICTIONARIES, SELECT, INSERT, ALTER, CREATE TABLE, CREATE VIEW, CREATE DICTIONARY, DROP TABLE, DROP VIEW, DROP DICTIONARY, TRUNCATE, OPTIMIZE, SYSTEM MERGES, SYSTEM TTL MERGES, SYSTEM FETCHES, SYSTEM MOVES, SYSTEM SENDS, SYSTEM REPLICATION QUEUES, SYSTEM DROP REPLICA, SYSTEM SYNC REPLICA, SYSTEM RESTART REPLICA, SYSTEM RESTORE REPLICA, SYSTEM FLUSH DISTRIBUTED, dictGet ON test.table TO B\n" + "SHOW GRANTS FOR B") == "GRANT SHOW TABLES, SHOW COLUMNS, SHOW DICTIONARIES, SELECT, INSERT, ALTER TABLE, ALTER VIEW, CREATE TABLE, CREATE VIEW, CREATE DICTIONARY, DROP TABLE, DROP VIEW, DROP DICTIONARY, TRUNCATE, OPTIMIZE, SYSTEM MERGES, SYSTEM TTL MERGES, SYSTEM FETCHES, SYSTEM MOVES, SYSTEM SENDS, SYSTEM REPLICATION QUEUES, SYSTEM DROP REPLICA, SYSTEM SYNC REPLICA, SYSTEM RESTART REPLICA, SYSTEM RESTORE REPLICA, SYSTEM FLUSH DISTRIBUTED, dictGet ON test.table TO B\n" instance.query("REVOKE ALL ON test.table FROM B", user='A') assert instance.query("SHOW GRANTS FOR B") == "" From 489a92c0679b51ed2f4b6b15ae634b6f30fa59b7 Mon Sep 17 00:00:00 2001 From: kssenii Date: Wed, 15 Sep 2021 18:22:33 +0300 Subject: [PATCH 13/13] Review fixes --- src/Databases/DatabaseAtomic.cpp | 1 - src/Databases/IDatabase.h | 2 +- .../PostgreSQL/DatabaseMaterializedPostgreSQL.cpp | 15 +++++++++++---- .../PostgreSQL/DatabaseMaterializedPostgreSQL.h | 2 +- src/Interpreters/InterpreterAlterQuery.cpp | 4 ++-- .../PostgreSQL/PostgreSQLReplicationHandler.cpp | 12 ++++++------ 6 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 7b1a8c6446e..2dbcd652004 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include #include diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index 92041b366a7..3cb1856d08d 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -282,7 +282,7 @@ public: /// Delete data and metadata stored inside the database, if exists. virtual void drop(ContextPtr /*context*/) {} - virtual void applyNewSettings(const SettingsChanges &, ContextPtr) + virtual void applySettingsChanges(const SettingsChanges &, ContextPtr) { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Database engine {} either does not support settings, or does not support altering settings", diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 42720fa4eb1..cb3cda8ab79 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -127,9 +127,11 @@ void DatabaseMaterializedPostgreSQL::loadStoredObjects( } -void DatabaseMaterializedPostgreSQL::applyNewSettings(const SettingsChanges & settings_changes, ContextPtr query_context) +void DatabaseMaterializedPostgreSQL::applySettingsChanges(const SettingsChanges & settings_changes, ContextPtr query_context) { std::lock_guard lock(handler_mutex); + bool need_update_on_disk = false; + for (const auto & change : settings_changes) { if (!settings->has(change.name)) @@ -140,12 +142,12 @@ void DatabaseMaterializedPostgreSQL::applyNewSettings(const SettingsChanges & se if (!query_context->isInternalQuery()) throw Exception(ErrorCodes::QUERY_NOT_ALLOWED, "Changing setting `{}` is not allowed", change.name); - DatabaseOnDisk::modifySettingsMetadata(settings_changes, query_context); + need_update_on_disk = true; } else if ((change.name == "materialized_postgresql_allow_automatic_update") || (change.name == "materialized_postgresql_max_block_size")) { - DatabaseOnDisk::modifySettingsMetadata(settings_changes, query_context); replication_handler->setSetting(change); + need_update_on_disk = true; } else { @@ -154,6 +156,9 @@ void DatabaseMaterializedPostgreSQL::applyNewSettings(const SettingsChanges & se settings->applyChange(change); } + + if (need_update_on_disk) + DatabaseOnDisk::modifySettingsMetadata(settings_changes, query_context); } @@ -185,6 +190,8 @@ StoragePtr DatabaseMaterializedPostgreSQL::tryGetTable(const String & name, Cont /// `except` is not empty in case it is detach and it will contain only one table name - name of detached table. +/// In case we have a user defined setting `materialized_postgresql_tables_list`, then list of tables is always taken there. +/// Otherwise we traverse materialized storages to find out the list. String DatabaseMaterializedPostgreSQL::getFormattedTablesList(const String & except) const { String tables_list; @@ -334,7 +341,7 @@ StoragePtr DatabaseMaterializedPostgreSQL::detachTable(const String & table_name auto nested = table_to_delete->as()->getNested(); if (!nested) - throw Exception(ErrorCodes::UNKNOWN_TABLE, "Inner table `{}` does not exist", table_name); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Inner table `{}` does not exist", table_name); std::lock_guard lock(handler_mutex); replication_handler->removeTableFromReplication(table_name); diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index effd0ec653a..a0f9b3fce7a 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -61,7 +61,7 @@ public: void stopReplication(); - void applyNewSettings(const SettingsChanges & settings_changes, ContextPtr query_context) override; + void applySettingsChanges(const SettingsChanges & settings_changes, ContextPtr query_context) override; void shutdown() override; diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index d0054439c56..6595e1c02be 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -189,7 +189,7 @@ BlockIO InterpreterAlterQuery::executeToDatabase(const ASTAlterQuery & alter) for (const auto & command : alter_commands) { if (command.type != AlterCommand::MODIFY_DATABASE_SETTING) - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Alter of type '{}' is not supported by databases", alterTypeToString(command.type)); + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Unsupported alter type for database engines"); } for (const auto & command : alter_commands) @@ -197,7 +197,7 @@ BlockIO InterpreterAlterQuery::executeToDatabase(const ASTAlterQuery & alter) if (!command.ignore) { if (command.type == AlterCommand::MODIFY_DATABASE_SETTING) - database->applyNewSettings(command.settings_changes, getContext()); + database->applySettingsChanges(command.settings_changes, getContext()); else throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Unsupported alter command"); } diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index 23ee7532b5e..456ca2c514e 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -119,6 +119,10 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) /// Data older than this is not available anymore. String snapshot_name, start_lsn; + /// Also lets have a separate non-replication connection, because we need two parallel transactions and + /// one connection can have one transaction at a time. + auto tmp_connection = std::make_shared(connection_info); + auto initial_sync = [&]() { LOG_TRACE(log, "Starting tables sync load"); @@ -136,15 +140,11 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) createReplicationSlot(tx, start_lsn, snapshot_name); } - /// Loading tables from snapshot requires a certain transaction type, so we need to open a new transactin. - /// But we cannot have more than one open transaciton on the same connection. Therefore we have - /// a separate connection to load tables. - postgres::Connection tmp_connection(connection_info); for (const auto & [table_name, storage] : materialized_storages) { try { - nested_storages[table_name] = loadFromSnapshot(tmp_connection, snapshot_name, table_name, storage->as ()); + nested_storages[table_name] = loadFromSnapshot(*tmp_connection, snapshot_name, table_name, storage->as()); } catch (Exception & e) { @@ -211,7 +211,7 @@ void PostgreSQLReplicationHandler::startSynchronization(bool throw_on_error) /// Handler uses it only for loadFromSnapshot and shutdown methods. consumer = std::make_shared( context, - std::make_shared(connection_info), + std::move(tmp_connection), replication_slot, publication_name, start_lsn,