diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index 47351f9eae1..e74da711c8b 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -938,8 +938,11 @@ void MergeTreeData::checkAlter(const AlterCommands & commands) continue; } + if (command.type == AlterCommand::RENAME_COLUMN || command.type == AlterCommand::MODIFY_COLUMN) + continue; + throw Exception( - "ALTER of key column " + command.column_name + " must be metadata-only", + "ALTER of key column " + command.column_name + " must be metadata-only, and command tyoe is " + std::to_string(command.type), ErrorCodes::ILLEGAL_COLUMN); } } diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index 386f5dee76e..96fc59822d2 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -365,9 +365,15 @@ void StorageMergeTree::alterInternal( else if (param.type == AlterCommand::RENAME_COLUMN) { rename_column = true; + if (param.primary_key != nullptr) + { + LOG_INFO(log, "old pk: " << *new_primary_key_ast); + new_primary_key_ast = param.primary_key; + LOG_INFO(log, "change to new pk: " << *new_primary_key_ast); + } if (params.size() != 1) { - throw Exception("There is an internal error for rename columns", ErrorCodes::LOGICAL_ERROR); + throw Exception("There is an internal error for rename columns, params size is " + std::to_string(params.size()) + ", but should be 1", ErrorCodes::LOGICAL_ERROR); } } } @@ -418,7 +424,7 @@ void StorageMergeTree::alterInternal( if (table_info) setTableInfo(table_info->get()); - if (primary_key_is_modified) + if (new_primary_key_ast != nullptr) { data.primary_expr_ast = new_primary_key_ast; } diff --git a/dbms/src/Storages/Transaction/SchemaBuilder.cpp b/dbms/src/Storages/Transaction/SchemaBuilder.cpp index c46fb0db7f6..8c392dba6a1 100644 --- a/dbms/src/Storages/Transaction/SchemaBuilder.cpp +++ b/dbms/src/Storages/Transaction/SchemaBuilder.cpp @@ -45,6 +45,25 @@ inline void setAlterCommandColumn(Logger * log, AlterCommand & command, const Co } } +AlterCommand newRenameColCommand(const String & old_col, const String & new_col, const TableInfo & orig_table_info) +{ + AlterCommand command; + command.type = AlterCommand::RENAME_COLUMN; + command.column_name = old_col; + command.new_column_name = new_col; + if (auto pk = orig_table_info.getPKHandleColumn()) + { + if (pk->get().name == old_col) + { + auto list = std::make_shared(); + auto new_pk = std::make_shared(new_col); + list->children.push_back(new_pk); + command.primary_key = list; + } + } + return command; +} + inline std::vector detectSchemaChanges(Logger * log, const TableInfo & table_info, const TableInfo & orig_table_info) { std::vector result; @@ -94,11 +113,7 @@ inline std::vector detectSchemaChanges(Logger * log, const TableI for (const auto & rename_pair : rename_result) { AlterCommands rename_commands; - AlterCommand command; - command.type = AlterCommand::RENAME_COLUMN; - command.column_name = rename_pair.first; - command.new_column_name = rename_pair.second; - rename_commands.push_back(command); + rename_commands.push_back(newRenameColCommand(rename_pair.first, rename_pair.second, orig_table_info)); result.push_back(rename_commands); } } @@ -247,19 +262,16 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) switch (diff.type) { case SchemaActionCreateTable: - case SchemaActionRecoverTable: - { + case SchemaActionRecoverTable: { newTableID = diff.table_id; break; } case SchemaActionDropTable: - case SchemaActionDropView: - { + case SchemaActionDropView: { oldTableID = diff.table_id; break; } - case SchemaActionTruncateTable: - { + case SchemaActionTruncateTable: { newTableID = diff.table_id; oldTableID = diff.old_table_id; break; @@ -267,24 +279,20 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) case SchemaActionAddColumn: case SchemaActionDropColumn: case SchemaActionModifyColumn: - case SchemaActionSetDefaultValue: - { + case SchemaActionSetDefaultValue: { applyAlterTable(di, diff.table_id); break; } - case SchemaActionRenameTable: - { + case SchemaActionRenameTable: { applyRenameTable(di, diff.old_schema_id, diff.table_id); break; } case SchemaActionAddTablePartition: - case SchemaActionDropTablePartition: - { + case SchemaActionDropTablePartition: { applyAlterPartition(di, diff.table_id); break; } - default: - { + default: { LOG_INFO(log, "ignore change type: " << int(diff.type)); break; } @@ -499,12 +507,14 @@ void SchemaBuilder::applyDropSchemaImpl(const String & database_name) drop_interpreter.execute(); } -String createTableStmt(const DBInfo & db_info, const TableInfo & table_info) +String createTableStmt(const DBInfo & db_info, const TableInfo & table_info, Logger * log) { + LOG_DEBUG(log, "Analyzing table info :" << table_info.serialize()); NamesAndTypes columns; std::vector pks; for (const auto & column : table_info.columns) { + LOG_DEBUG(log, "Analyzing column :" + column.name + " type " + std::to_string((int)column.tp)); DataTypePtr type = getDataTypeByColumnInfo(column); columns.emplace_back(NameAndTypePair(column.name, type)); @@ -553,9 +563,15 @@ String createTableStmt(const DBInfo & db_info, const TableInfo & table_info) template void SchemaBuilder::applyCreatePhysicalTableImpl(const TiDB::DBInfo & db_info, TiDB::TableInfo & table_info) { + if (table_info.is_view) + { + LOG_INFO(log, "Table " << table_info.name << " is a view table, ignore it."); + return; + } + table_info.schema_version = target_version; - String stmt = createTableStmt(db_info, table_info); + String stmt = createTableStmt(db_info, table_info, log); LOG_INFO(log, "try to create table with stmt: " << stmt); diff --git a/dbms/src/Storages/Transaction/SchemaSyncService.cpp b/dbms/src/Storages/Transaction/SchemaSyncService.cpp index 923cfab13e1..2b4fc1578fe 100644 --- a/dbms/src/Storages/Transaction/SchemaSyncService.cpp +++ b/dbms/src/Storages/Transaction/SchemaSyncService.cpp @@ -17,7 +17,7 @@ SchemaSyncService::SchemaSyncService(DB::Context & context_) } catch (const Exception & e) { - LOG_WARNING(log, "Schema sync failed by " << e.message()); + LOG_WARNING(log, "Schema sync failed by " << e.displayText() << " \n stack : " << e.getStackTrace().toString()); } return false; }, diff --git a/dbms/src/Storages/Transaction/TiDB.cpp b/dbms/src/Storages/Transaction/TiDB.cpp index 96bf3e2df08..437f2ceed1b 100644 --- a/dbms/src/Storages/Transaction/TiDB.cpp +++ b/dbms/src/Storages/Transaction/TiDB.cpp @@ -399,6 +399,10 @@ void TableInfo::deserialize(const String & json_str) try { schema_version = obj->getValue("schema_version"); } + if (obj->has("view") && !obj->getObject("view").isNull()) + { + is_view = true; + } } catch (const Poco::Exception & e) { diff --git a/dbms/src/Storages/Transaction/TiDB.h b/dbms/src/Storages/Transaction/TiDB.h index f5ae2ac2aaf..5e0b09d0d3b 100644 --- a/dbms/src/Storages/Transaction/TiDB.h +++ b/dbms/src/Storages/Transaction/TiDB.h @@ -272,6 +272,8 @@ struct TableInfo bool is_partition_table = false; TableID belonging_table_id = -1; PartitionInfo partition; + // If the table is view, we should ignore it. + bool is_view = false; Int64 schema_version = -1; ColumnID getColumnID(const String & name) const; diff --git a/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h b/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h index 04969d74ece..a0d2260b61e 100644 --- a/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h +++ b/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h @@ -78,6 +78,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer loadAllSchema(getter, version, context); } cur_version = version; + LOG_INFO(log, "end sync schema, version has been updated to " + std::to_string(cur_version)); return true; } @@ -109,7 +110,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer } catch (Exception & e) { - LOG_ERROR(log, "apply diff meets exception : " + e.displayText()); + LOG_ERROR(log, "apply diff meets exception : " << e.displayText() << " \n stack is " << e.getStackTrace().toString()); return false; } return true;