dbms: FunctionCast for Enums: allow changing names but not values for existing elements [#METR-19265]

dbms/include/DB/Functions/FunctionsConversion.h

@@ -22,7 +22,7 @@
 #include <DB/DataTypes/DataTypeTuple.h>
 #include <ext/enumerate.hpp>
 #include <ext/collection_cast.hpp>
-#include "FunctionsMiscellaneous.h"
+#include <DB/Functions/FunctionsMiscellaneous.h>
 
 
 namespace DB
@@ -1688,32 +1688,41 @@ class FunctionCast final : public IFunction
 
 		using ValueType = std::common_type_t<typename EnumTypeFrom::FieldType, typename EnumTypeTo::FieldType>;
 		using NameValuePair = std::pair<std::string, ValueType>;
+		using EnumValues = std::vector<NameValuePair>;
 
-		struct CompareNameValuePairSecond final
-		{
-			bool operator()(const NameValuePair & lhs, const NameValuePair & rhs) const
-			{
-				return lhs.second < rhs.second;
-			}
-		};
+//		EnumValues value_intersection;
+//		std::set_intersection(std::begin(from_values), std::end(from_values),
+//			std::begin(to_values), std::end(to_values), std::back_inserter(value_intersection),
+//			[] (auto && from, auto && to) { return from.second < to.second; });
+//
+//		for (const auto & name_value : value_intersection)
+//		{
+//			const auto & old_name = name_value.first;
+//			const auto & new_name = to_type->getNameForValue(name_value.second);
+//			if (old_name != new_name)
+//				throw Exception{
+//					"Enum conversion changes name for value " + toString(name_value.second) +
+//						" from '" + old_name + "' to '" + new_name + "'",
+//					ErrorCodes::CANNOT_CONVERT_TYPE
+//				};
+//		}
 
-		std::set<NameValuePair, CompareNameValuePairSecond> isection;
+		EnumValues name_intersection;
 		std::set_intersection(std::begin(from_values), std::end(from_values),
-			std::begin(to_values), std::end(to_values), std::inserter(isection, std::end(isection)),
-			CompareNameValuePairSecond{});
+			std::begin(to_values), std::end(to_values), std::back_inserter(name_intersection),
+			[] (auto && from, auto && to) { return from.first < to.first; });
 
-		if (!isection.empty())
-			for (const auto & name_value : isection)
-			{
-				const auto & old_name = from_type->getNameForValue(name_value.second);
-				const auto & new_name = to_type->getNameForValue(name_value.second);
-				if (old_name != new_name)
-					throw Exception{
-						"Enum conversion changes name for value " + toString(name_value.second) +
-							" from " + old_name + " to " + new_name,
-						ErrorCodes::CANNOT_CONVERT_TYPE
-					};
-			}
+		for (const auto & name_value : name_intersection)
+		{
+			const auto & old_value = name_value.second;
+			const auto & new_value = to_type->getValue(name_value.first);
+			if (old_value != new_value)
+				throw Exception{
+					"Enum conversion changes value for element '" + name_value.first +
+						"' from " + toString(old_value) + " to " + toString(new_value),
+					ErrorCodes::CANNOT_CONVERT_TYPE
+				};
+		}
 	};
 
 	template <typename ColumnStringType, typename EnumType>

dbms/include/DB/Storages/MergeTree/MergeTreeData.h

@@ -891,11 +891,13 @@ private:
 	/** Выражение, преобразующее типы столбцов.
 	  * Если преобразований типов нет, out_expression=nullptr.
 	  * out_rename_map отображает файлы-столбцы на выходе выражения в новые файлы таблицы.
+	  * out_force_update_metadata показывает, нужно ли обновить метаданные даже если out_rename_map пуста (используется
+	  * 	для бесплатного изменения списка значений Enum).
 	  * Файлы, которые нужно удалить, в out_rename_map отображаются в пустую строку.
 	  * Если !part, просто проверяет, что все нужные преобразования типов допустимы.
 	  */
 	void createConvertExpression(const DataPartPtr & part, const NamesAndTypesList & old_columns, const NamesAndTypesList & new_columns,
-		ExpressionActionsPtr & out_expression, NameToNameMap & out_rename_map);
+		ExpressionActionsPtr & out_expression, NameToNameMap & out_rename_map, bool & out_force_update_metadata);
 
 	/// Рассчитывает размеры столбцов в сжатом виде для текущего состояния data_parts
 	void calculateColumnSizes();

dbms/src/Storages/MergeTree/MergeTreeData.cpp

@@ -435,18 +435,20 @@ void MergeTreeData::checkAlter(const AlterCommands & params)
 	/// Проверим, что преобразования типов возможны.
 	ExpressionActionsPtr unused_expression;
 	NameToNameMap unused_map;
+	bool unused_bool;
 
 	/// augment plain columns with materialized columns for convert expression creation
 	new_columns.insert(std::end(new_columns),
 		std::begin(new_materialized_columns), std::end(new_materialized_columns));
-	createConvertExpression(nullptr, getColumnsList(), new_columns, unused_expression, unused_map);
+	createConvertExpression(nullptr, getColumnsList(), new_columns, unused_expression, unused_map, unused_bool);
 }
 
 void MergeTreeData::createConvertExpression(const DataPartPtr & part, const NamesAndTypesList & old_columns, const NamesAndTypesList & new_columns,
-	ExpressionActionsPtr & out_expression, NameToNameMap & out_rename_map)
+	ExpressionActionsPtr & out_expression, NameToNameMap & out_rename_map, bool & out_force_update_metadata)
 {
 	out_expression = nullptr;
-	out_rename_map.clear();
+	out_rename_map = {};
+	out_force_update_metadata = false;
 
 	typedef std::map<String, DataTypePtr> NameToType;
 	NameToType new_types;
@@ -494,15 +496,17 @@ void MergeTreeData::createConvertExpression(const DataPartPtr & part, const Name
 			const String new_type_name = new_type->getName();
 			const auto & old_type = column.type;
 
-			/// if this is not a check (part != nullptr) and alter does not change enum underlying type - do nothing
-			if (part && typeid(*new_type) == typeid(*old_type) &&
-				(typeid_cast<const DataTypeEnum8 *>(new_type) || typeid_cast<const DataTypeEnum16 *>(new_type)))
-				continue;
-			else if (new_type_name != old_type->getName() &&
-				(!part || part->hasColumnFiles(column.name)))
+			if (new_type_name != old_type->getName() && (!part || part->hasColumnFiles(column.name)))
 			{
-				/// Нужно изменить тип столбца.
+				// При ALTER между Enum с одинаковым подлежащим типом столбцы не трогаем, лишь просим обновить columns.txt
+				if (part && typeid(*new_type) == typeid(*old_type) &&
+					(typeid_cast<const DataTypeEnum8 *>(new_type) || typeid_cast<const DataTypeEnum16 *>(new_type)))
+				{
+					out_force_update_metadata = true;
+					continue;
+				}
 
+				/// Нужно изменить тип столбца.
 				if (!out_expression)
 					out_expression = new ExpressionActions(NamesAndTypesList(), context.getSettingsRef());
 
@@ -538,7 +542,8 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 {
 	ExpressionActionsPtr expression;
 	AlterDataPartTransactionPtr transaction(new AlterDataPartTransaction(part)); /// Блокирует изменение куска.
-	createConvertExpression(part, part->columns, new_columns, expression, transaction->rename_map);
+	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() > 5)
 	{
@@ -548,7 +553,7 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::alterDataPart(
 						+ ". Aborting just in case");
 	}
 
-	if (transaction->rename_map.empty())
+	if (transaction->rename_map.empty() && !force_update_metadata)
 	{
 		transaction->clear();
 		return nullptr;