From 97c2579fb9da90d9e5ee8d018abb12faa7801703 Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Wed, 30 Sep 2020 22:50:23 +0300
Subject: [PATCH] Use separate function

---
 src/Storages/MergeTree/MergeTreeData.cpp | 73 +++++++++++++-----------
 1 file changed, 40 insertions(+), 33 deletions(-)

diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp
index 0aee7a25875..3f09f3a7347 100644
--- a/src/Storages/MergeTree/MergeTreeData.cpp
+++ b/src/Storages/MergeTree/MergeTreeData.cpp
@@ -1330,6 +1330,44 @@ bool isSafeForPartitionKeyConversion(const IDataType * from, const IDataType * t
     return false;
 }
 
+/// Special check for alters of VersionedCollapsingMergeTree version column
+void checkVersionColumnTypesConversion(const IDataType * old_type, const IDataType * new_type, const String column_name)
+{
+    /// Check new type can be used as version
+    if (!new_type->canBeUsedAsVersion())
+        throw Exception("Cannot alter version column " + backQuoteIfNeed(column_name) +
+            " to type " + new_type->getName() +
+            " because version column must be of an integer type or of type Date or DateTime"
+            , ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN);
+
+    auto which_new_type = WhichDataType(new_type);
+    auto which_old_type = WhichDataType(old_type);
+
+    /// Check alter to different sign or float -> int and so on
+    if ((which_old_type.isInt() && !which_new_type.isInt())
+        || (which_old_type.isUInt() && !which_new_type.isUInt())
+        || (which_old_type.isDate() && !which_new_type.isDate())
+        || (which_old_type.isDateTime() && !which_new_type.isDateTime())
+        || (which_old_type.isFloat() && !which_new_type.isFloat()))
+    {
+        throw Exception("Cannot alter version column " + backQuoteIfNeed(column_name) +
+            " from type " + old_type->getName() +
+            " to type " + new_type->getName() + " because new type will change sort order of version column." +
+            " The only possible conversion is expansion of the number of bytes of the current type."
+            , ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN);
+    }
+
+    /// Check alter to smaller size: UInt64 -> UInt32 and so on
+    if (new_type->getSizeOfValueInMemory() < old_type->getSizeOfValueInMemory())
+    {
+        throw Exception("Cannot alter version column " + backQuoteIfNeed(column_name) +
+            " from type " + old_type->getName() +
+            " to type " + new_type->getName() + " because new type is smaller than current in the number of bytes." +
+            " The only possible conversion is expansion of the number of bytes of the current type."
+            , ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN);
+    }
+}
+
 }
 
 void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const Settings & settings) const
@@ -1421,41 +1459,10 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S
         {
             auto new_type = command.data_type;
             auto old_type = old_types[command.column_name];
-            /// Check new type can be used as version
-            if (!new_type->canBeUsedAsVersion())
-                throw Exception("Cannot alter version column " + backQuoteIfNeed(command.column_name) +
-                    " to type " + new_type->getName() +
-                    " because version column must be of an integer type or of type Date or DateTime"
-                    , ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN);
 
-            auto which_new_type = WhichDataType(new_type);
-            auto which_old_type = WhichDataType(old_type);
+            checkVersionColumnTypesConversion(old_type, new_type.get(), command.column_name);
 
-            /// Check alter to different sign or float -> int and so on
-            if ((which_old_type.isInt() && !which_new_type.isInt())
-                || (which_old_type.isUInt() && !which_new_type.isUInt())
-                || (which_old_type.isDate() && !which_new_type.isDate())
-                || (which_old_type.isDateTime() && !which_new_type.isDateTime())
-                || (which_old_type.isFloat() && !which_new_type.isFloat()))
-            {
-                throw Exception("Cannot alter version column " + backQuoteIfNeed(command.column_name) +
-                    " from type " + old_type->getName() +
-                    " to type " + new_type->getName() + " because new type will change sort order of version column." +
-                    " The only possible conversion is expansion of the number of bytes of the current type."
-                    , ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN);
-            }
-
-            /// Check alter to smaller size: UInt64 -> UInt32 and so on
-            if (new_type->getSizeOfValueInMemory() < old_type->getSizeOfValueInMemory())
-            {
-                throw Exception("Cannot alter version column " + backQuoteIfNeed(command.column_name) +
-                    " from type " + old_type->getName() +
-                    " to type " + new_type->getName() + " because new type is smaller than current in the number of bytes." +
-                    " The only possible conversion is expansion of the number of bytes of the current type."
-                    , ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN);
-            }
-
-            /// Positive case, alter allowed
+            /// No other checks required
             continue;
         }