Alter primary key: development [#METR-21119].

dbms/src/Parsers/ParserAlterQuery.cpp

@@ -267,8 +267,17 @@ bool ParserAlterQuery::parseImpl(Pos & pos, Pos end, ASTPtr & node, Pos & max_pa
 
 				ws.ignore(pos, end);
 
-				ParserParenthesisExpression parser_tuple;
+				if (!ParserString("(").ignore(pos, end, max_parsed_pos, expected))
-				if (!parser_tuple.parse(pos, end, params.primary_key, max_parsed_pos, expected))
+					return false;
+
+				ws.ignore(pos, end);
+
+				if (!ParserNotEmptyExpressionList(false).parse(pos, end, params.primary_key, max_parsed_pos, expected))
+					return false;
+
+				ws.ignore(pos, end);
+
+				if (!ParserString(")").ignore(pos, end, max_parsed_pos, expected))
 					return false;
 
 				ws.ignore(pos, end);

dbms/src/Storages/MergeTree/MergeTreeData.cpp

@@ -652,7 +652,20 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 	bool force_update_metadata;
 	createConvertExpression(part, part->columns, new_columns, expression, transaction->rename_map, force_update_metadata);
 
+	if (!skip_sanity_checks && transaction->rename_map.size() > settings.max_files_to_modify_in_alter_columns)
+	{
+		transaction->clear();
+
+		throw Exception("Suspiciously many (" + toString(transaction->rename_map.size())
+		+ ") files need to be modified in part " + part->name + ". Aborting just in case");
+	}
+
+	DataPart::Checksums add_checksums;
+
 	/// Обновление первичного ключа, если нужно.
+	size_t new_primary_key_file_size{};
+	uint128 new_primary_key_hash{};
+
 	if (new_primary_key.get() != primary_expr_ast.get())
 	{
 		ExpressionActionsPtr new_primary_expr = ExpressionAnalyzer(new_primary_key, context, nullptr, getColumnsList()).getActions(true);
@@ -682,7 +695,7 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 			else
 			{
 				const IDataType & type = *new_primary_key_sample.getByPosition(i).type;
-				new_index[i] = type.createConstColumn(part->size, type.getDefault());
+				new_index[i] = type.createConstColumn(part->size, type.getDefault())->convertToFullColumnIfConst();
 			}
 		}
 
@@ -691,20 +704,17 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 
 		String index_tmp_path = full_path + part->name + "/primary.idx.tmp";
 		WriteBufferFromFile index_file(index_tmp_path);
+		HashingWriteBuffer index_stream(index_file);
 
 		for (size_t i = 0, size = part->size; i < size; ++i)
 			for (size_t j = 0; j < new_key_size; ++j)
-				new_primary_key_sample.unsafeGetByPosition(j).type.get()->serializeBinary(*new_index[j].get(), index_file);
+				new_primary_key_sample.unsafeGetByPosition(j).type.get()->serializeBinary(*new_index[j].get(), i, index_stream);
 
 		transaction->rename_map["primary.idx.tmp"] = "primary.idx";
-	}
 
-	if (!skip_sanity_checks && transaction->rename_map.size() > settings.max_files_to_modify_in_alter_columns)
+		index_stream.next();
-	{
+		new_primary_key_file_size = index_stream.count();
-		transaction->clear();
+		new_primary_key_hash = index_stream.getHash();
-
-		throw Exception("Suspiciously many (" + toString(transaction->rename_map.size())
-			+ ") files need to be modified in part " + part->name + ". Aborting just in case");
 	}
 
 	if (transaction->rename_map.empty() && !force_update_metadata)
@@ -713,8 +723,6 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 		return nullptr;
 	}
 
-	DataPart::Checksums add_checksums;
-
 	/// Применим выражение и запишем результат во временные файлы.
 	if (expression)
 	{
@@ -740,13 +748,15 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 	for (auto it : transaction->rename_map)
 	{
 		if (it.second == "")
-		{
 			new_checksums.files.erase(it.first);
-		}
 		else
-		{
 			new_checksums.files[it.second] = add_checksums.files[it.first];
-		}
+	}
+
+	if (new_primary_key_file_size)
+	{
+		new_checksums.files["primary.idx"].file_size = new_primary_key_file_size;
+		new_checksums.files["primary.idx"].file_hash = new_primary_key_hash;
 	}
 
 	/// Запишем обновленные контрольные суммы во временный файл

dbms/src/Storages/StorageMergeTree.cpp

@@ -209,10 +209,9 @@ void StorageMergeTree::alter(
 
 	auto table_hard_lock = lockStructureForAlter();
 
-	context.getDatabase(database_name)->alterTable(
+	IDatabase::ASTModifier engine_modifier;
-		context, table_name,
+	if (primary_key_is_modified)
-		new_columns, new_materialized_columns, new_alias_columns, new_column_defaults,
+		engine_modifier = [&new_primary_key_ast] (ASTPtr & engine_ast)
-		[&new_primary_key_ast] (ASTPtr & engine_ast)
 		{
 			ASTFunction * tuple = new ASTFunction(new_primary_key_ast->range);
 			tuple->name = "tuple";
@@ -220,8 +219,14 @@ void StorageMergeTree::alter(
 			tuple->children.push_back(tuple->arguments);
 
 			/// Первичный ключ находится на втором месте в описании движка таблицы и может быть представлен в виде кортежа.
+			/// TODO: Не всегда на втором месте. Если есть ключ сэмплирования, то на третьем. Исправить.
 			typeid_cast<ASTExpressionList &>(*typeid_cast<ASTFunction &>(*engine_ast).arguments).children.at(1) = tuple;
-		});
+		};
+
+	context.getDatabase(database_name)->alterTable(
+		context, table_name,
+		new_columns, new_materialized_columns, new_alias_columns, new_column_defaults,
+		engine_modifier);
 
 	materialized_columns = new_materialized_columns;
 	alias_columns = new_alias_columns;