From c1e973b609f5ea6af3af2b6ef2527c49b5b93d90 Mon Sep 17 00:00:00 2001
From: tsy <tangsiyang2001@foxmail.com>
Date: Fri, 2 Aug 2024 20:58:45 +0800
Subject: [PATCH] [fix](schema-change) Fix schema change run into nullable
 check specified for MV

---
 be/src/olap/schema_change.cpp                 |  3 +-
 .../apache/doris/alter/SchemaChangeJobV2.java | 15 ++++----
 .../schema_change/test_column_reorder.groovy  | 38 +++++++++++++++++++
 3 files changed, 47 insertions(+), 9 deletions(-)
 create mode 100644 regression-test/suites/variant_p0/schema_change/test_column_reorder.groovy

diff --git a/be/src/olap/schema_change.cpp b/be/src/olap/schema_change.cpp
index 599d9c1d1423ca..38dbcf1c429bb4 100644
--- a/be/src/olap/schema_change.cpp
+++ b/be/src/olap/schema_change.cpp
@@ -291,6 +291,7 @@ Status BlockChanger::change_block(vectorized::Block* ref_block,
     // swap ref_block[key] and new_block[value]
     std::list<std::pair<int, int>> swap_idx_list;
     for (int idx = 0; idx < column_size; idx++) {
+        // just for MV, schema change should not run into this branch
         if (_schema_mapping[idx].expr != nullptr) {
             vectorized::VExprContextSPtr ctx;
             RETURN_IF_ERROR(vectorized::VExpr::create_expr_tree(*_schema_mapping[idx].expr, ctx));
@@ -367,7 +368,7 @@ Status BlockChanger::change_block(vectorized::Block* ref_block,
     return Status::OK();
 }
 
-// This check is to prevent schema-change from causing data loss
+// This check is for MV to prevent schema-change from causing data loss
 Status BlockChanger::_check_cast_valid(vectorized::ColumnPtr ref_column,
                                        vectorized::ColumnPtr new_column, AlterTabletType type) {
     if (ref_column->size() != new_column->size()) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
index 527c8620c6cf77..111bfbce1f7fab 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
@@ -79,6 +79,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
@@ -480,14 +481,10 @@ protected void runWaitingTxnJob() throws AlterCancelException {
                         if (indexColumnMap.containsKey(SchemaChangeHandler.SHADOW_NAME_PREFIX + column.getName())) {
                             Column newColumn = indexColumnMap.get(
                                     SchemaChangeHandler.SHADOW_NAME_PREFIX + column.getName());
-                            if (newColumn.getType() != column.getType()) {
-                                try {
-                                    SlotRef slot = new SlotRef(destSlotDesc);
-                                    slot.setCol(column.getName());
-                                    defineExprs.put(column.getName(), slot.castTo(newColumn.getType()));
-                                } catch (AnalysisException e) {
-                                    throw new AlterCancelException(e.getMessage());
-                                }
+                            if (!Objects.equals(newColumn.getType(), column.getType())) {
+                                SlotRef slot = new SlotRef(destSlotDesc);
+                                slot.setCol(column.getName());
+                                defineExprs.put(column.getName(), slot.castTo(newColumn.getType()));
                             }
                         }
                     }
@@ -510,6 +507,8 @@ protected void runWaitingTxnJob() throws AlterCancelException {
                 }
 
             } // end for partitions
+        } catch (AnalysisException e) {
+            throw new AlterCancelException(e.getMessage());
         } finally {
             tbl.readUnlock();
         }
diff --git a/regression-test/suites/variant_p0/schema_change/test_column_reorder.groovy b/regression-test/suites/variant_p0/schema_change/test_column_reorder.groovy
new file mode 100644
index 00000000000000..bb1e4137189b56
--- /dev/null
+++ b/regression-test/suites/variant_p0/schema_change/test_column_reorder.groovy
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_column_reorder") {
+    def tableName = "variant_column_reorder"
+    sql "DROP TABLE IF EXISTS ${tableName}"
+    sql """
+        CREATE TABLE IF NOT EXISTS ${tableName} (
+            k bigint,
+            v variant
+        )
+        DUPLICATE KEY(`k`)
+        DISTRIBUTED BY HASH(k) BUCKETS 4
+        properties("replication_num" = "1");
+    """
+    sql """INSERT INTO ${tableName} SELECT *, '{"k1":1, "k2": "hello world", "k3" : [1234], "k4" : 1.10000, "k5" : [[123]]}' FROM numbers("number" = "1")"""
+    sql """alter table ${tableName} add column t2 datetime default null"""
+    sql """alter table ${tableName} modify column v variant  after t2"""
+
+    waitForSchemaChangeDone {
+        sql """SHOW ALTER TABLE COLUMN WHERE IndexName='${tableName}' ORDER BY createtime DESC LIMIT 1"""
+        time 600
+    }
+}
\ No newline at end of file