[Bug](schame-change) fix wrong result after reorder mor table

be/src/olap/column_mapping.h

@@ -27,16 +27,18 @@ namespace doris {
 class WrapperField;
 
 struct ColumnMapping {
-    ColumnMapping() : ref_column(-1), default_value(nullptr) {}
+    ColumnMapping() = default;
     virtual ~ColumnMapping() = default;
 
+    bool has_reference() const { return expr != nullptr || ref_column >= 0; }
+
     // <0: use default value
     // >=0: use origin column
-    int32_t ref_column;
+    int32_t ref_column = -1;
     // normally for default value. stores values for filters
-    WrapperField* default_value;
+    WrapperField* default_value = nullptr;
     std::shared_ptr<TExpr> expr;
-    const TabletColumn* new_column;
+    const TabletColumn* new_column = nullptr;
 };
 
 using SchemaMapping = std::vector<ColumnMapping>;

be/src/olap/schema_change.cpp

@@ -921,13 +921,6 @@ Status SchemaChangeHandler::_do_process_alter_tablet_v2(const TAlterTabletReqV2&
             for (auto item : request.materialized_view_params) {
                 AlterMaterializedViewParam mv_param;
                 mv_param.column_name = item.column_name;
-                /*
-                 * origin_column_name is always be set now,
-                 * but origin_column_name may be not set in some materialized view function. eg:count(1)
-                */
-                if (item.__isset.origin_column_name) {
-                    mv_param.origin_column_name = item.origin_column_name;
-                }
 
                 if (item.__isset.mv_expr) {
                     mv_param.expr = std::make_shared<TExpr>(item.mv_expr);
@@ -1169,9 +1162,7 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
         if (materialized_function_map.find(column_name) != materialized_function_map.end()) {
             auto mvParam = materialized_function_map.find(column_name)->second;
             column_mapping->expr = mvParam.expr;
-            int32_t column_index = base_tablet_schema->field_index(mvParam.origin_column_name);
-            if (column_index >= 0) {
-                column_mapping->ref_column = column_index;
+            if (column_mapping->expr != nullptr) {
                 continue;
             } else if (sc_params.alter_tablet_type != ROLLUP) {
                 return Status::Error<CE_CMD_PARAMS_ERROR>(
@@ -1209,15 +1200,13 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
         changer->set_where_expr(materialized_function_map.find(WHERE_SIGN)->second.expr);
     }
 
-    // Check if re-aggregation is needed.
-    *sc_sorting = false;
     // If the reference sequence of the Key column is out of order, it needs to be reordered
     int num_default_value = 0;
 
     for (int i = 0, new_schema_size = new_tablet->num_key_columns(); i < new_schema_size; ++i) {
         ColumnMapping* column_mapping = changer->get_mutable_column_mapping(i);
 
-        if (column_mapping->ref_column < 0) {
+        if (!column_mapping->has_reference()) {
             num_default_value++;
             continue;
         }
@@ -1252,6 +1241,11 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
         return Status::OK();
     }
 
+    if (sc_params.alter_tablet_type == ROLLUP) {
+        *sc_directly = true;
+        return Status::OK();
+    }
+
     if (new_tablet->enable_unique_key_merge_on_write() &&
         new_tablet->num_key_columns() > base_tablet_schema->num_key_columns()) {
         *sc_directly = true;
@@ -1264,36 +1258,18 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
         return Status::OK();
     }
 
-    for (size_t i = 0; i < new_tablet->num_columns(); ++i) {
-        ColumnMapping* column_mapping = changer->get_mutable_column_mapping(i);
-        if (column_mapping->ref_column < 0) {
-            continue;
-        } else {
-            auto column_new = new_tablet_schema->column(i);
-            auto column_old = base_tablet_schema->column(column_mapping->ref_column);
-            if (column_new.type() != column_old.type() ||
-                column_new.precision() != column_old.precision() ||
-                column_new.frac() != column_old.frac() ||
-                column_new.length() != column_old.length() ||
-                column_new.is_bf_column() != column_old.is_bf_column() ||
-                column_new.has_bitmap_index() != column_old.has_bitmap_index() ||
-                new_tablet_schema->has_inverted_index(column_new.unique_id()) !=
-                        base_tablet_schema->has_inverted_index(column_old.unique_id())) {
-                *sc_directly = true;
-                return Status::OK();
-            }
-        }
-    }
-
     if (!sc_params.delete_handler->empty()) {
         // there exists delete condition in header, can't do linked schema change
         *sc_directly = true;
+        return Status::OK();
     }
 
-    if (base_tablet->tablet_meta()->preferred_rowset_type() !=
-        new_tablet->tablet_meta()->preferred_rowset_type()) {
-        // If the base_tablet and new_tablet rowset types are different, just use directly type
-        *sc_directly = true;
+    for (size_t i = 0; i < new_tablet->num_columns(); ++i) {
+        ColumnMapping* column_mapping = changer->get_mutable_column_mapping(i);
+        if (column_mapping->expr != nullptr) {
+            *sc_directly = true;
+            return Status::OK();
+        }
     }
 
     // if rs_reader has remote files, link schema change is not supported,

regression-test/data/mv_p0/multi_slot_k1a2p2ap3p/multi_slot_k1a2p2ap3p.out

@@ -2,12 +2,18 @@
 -- !select_star --
 -4	-4	-4	d
 1	1	1	a
+1	1	1	a
+2	2	2	b
 2	2	2	b
 3	-3	\N	c
+3	-3	\N	c
 
 -- !select_mv --
 1	\N
+1	\N
 1	1
 3	7
+3	7
+5	9
 5	9
 

regression-test/data/schema_change_p0/reorder_mor/reorder_mor.out

@@ -0,0 +1,7 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !select_star --
+1	3	5
+2	4	3
+8	1	3
+9	2	3
+

regression-test/suites/mv_p0/multi_slot_k1a2p2ap3p/multi_slot_k1a2p2ap3p.groovy

@@ -36,6 +36,8 @@ suite ("multi_slot_k1a2p2ap3p") {
     sql "insert into d_table select 2,2,2,'b';"
     sql "insert into d_table select 3,-3,null,'c';"
 
+    sql "insert into d_table values(1,1,1,'a'),(2,2,2,'b'),(3,-3,null,'c');"
+
     createMV ("create materialized view k1a2p2ap3p as select abs(k1)+k2+1,abs(k2+2)+k3+3 from d_table;")
 
     sql "insert into d_table select -4,-4,-4,'d';"

regression-test/suites/schema_change_p0/reorder_mor/reorder_mor.groovy

@@ -0,0 +1,52 @@
+// 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.
+
+import org.codehaus.groovy.runtime.IOGroovyMethods
+
+suite ("reorder_mor") {
+
+    sql """ DROP TABLE IF EXISTS test; """
+
+    sql """
+            create table test (c0 int, c1 int, c2 int) ENGINE=OLAP UNIQUE KEY(`c0`, c1) COMMENT 'OLAP' DISTRIBUTED BY HASH(`c0`) BUCKETS 1 PROPERTIES ("replication_allocation" = "tag.location.default: 1", "enable_unique_key_merge_on_write"="false");
+        """
+
+    sql "insert into test values(1,8,3),(2,9,3),(3,1,3),(4,2,3);"
+
+    sql "alter table test order by(c1,c0,c2);"
+
+    def getJobState = { tableName ->
+         def jobStateResult = sql """  SHOW ALTER TABLE COLUMN WHERE IndexName='${tableName}' ORDER BY createtime DESC LIMIT 1 """
+         return jobStateResult[0][9]
+    }
+
+    int max_try_time = 100
+    while (max_try_time--){
+        String result = getJobState("test")
+        if (result == "FINISHED") {
+            sleep(1000)
+            break
+        } else {
+            sleep(1000)
+            assertTrue(max_try_time>1)
+        }
+    }
+
+    sql "insert into test values(1, 3, 5);"
+
+    qt_select_star "select * from test order by 1,2,3;"
+}