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

…#29045)

* fix wrong result after reorder mor table

* update

be/src/olap/column_mapping.h

@@ -30,6 +30,8 @@ struct ColumnMapping {
     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 = -1;

be/src/olap/schema_change.cpp

@@ -1181,11 +1181,6 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
     const std::unordered_map<std::string, AlterMaterializedViewParam>& materialized_function_map =
             sc_params.materialized_params_map;
     DescriptorTbl desc_tbl = *sc_params.desc_tbl;
-
-    if (sc_params.alter_tablet_type == ROLLUP) {
-        *sc_directly = true;
-    }
-
     TabletSchemaSPtr new_tablet_schema = sc_params.new_tablet_schema;
 
     // set column mapping
@@ -1246,15 +1241,13 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
         changer->set_where_expr(materialized_function_map.find(WHERE_SIGN_LOWER)->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->expr == nullptr) {
+        if (!column_mapping->has_reference()) {
             num_default_value++;
             continue;
         }
@@ -1288,6 +1281,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;
@@ -1300,25 +1298,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->expr == nullptr) {
-            continue;
-        } else {
-            *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/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/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;"
+}