You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2023/04/30 14:46:45 UTC

[doris] branch master updated: [fix](schema_change) remove shadow prefix of schema for tablesink (#18822)

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new a978be32a6 [fix](schema_change) remove shadow prefix of schema for tablesink (#18822)
a978be32a6 is described below

commit a978be32a6c8cd96d01f1b9e728648e68f89dcbd
Author: Yongqiang YANG <98...@users.noreply.github.com>
AuthorDate: Sun Apr 30 22:46:36 2023 +0800

    [fix](schema_change) remove shadow prefix of schema for tablesink (#18822)
    
    LSC updates tablet's schema in writing. Be optimized adding columns via linked schema change and
    it distinguishes adding by comparing column name. e.g. if new column's name is not found in old schema,
    then it is a newly-add column.
    
    When a table is under schema-changing, it adds __doris_shadow_ prefix in name of columns in shadow index.
    Then  writes during schema-changing would bring schema with __doris_shadow_ to be.
    If schema change request arrives at be after writes, then be do it as a add-column schema change due to
    __doris_shadow_ is not in base tablet.
---
 be/src/olap/schema_change.cpp                                | 12 +++++++++---
 be/src/olap/tablet.h                                         |  2 ++
 .../java/org/apache/doris/alter/SchemaChangeHandler.java     |  1 +
 .../src/main/java/org/apache/doris/catalog/Column.java       |  4 ++++
 .../main/java/org/apache/doris/planner/OlapTableSink.java    |  2 +-
 .../decimalv3/test_agg_keys_schema_change_decimalv3.groovy   |  1 +
 6 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/be/src/olap/schema_change.cpp b/be/src/olap/schema_change.cpp
index ec56516211..3bf822c7c2 100644
--- a/be/src/olap/schema_change.cpp
+++ b/be/src/olap/schema_change.cpp
@@ -1704,7 +1704,7 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
     for (int i = 0, new_schema_size = new_tablet->tablet_schema()->num_columns();
          i < new_schema_size; ++i) {
         const TabletColumn& new_column = new_tablet->tablet_schema()->column(i);
-        const string& column_name = new_column.name();
+        const std::string& column_name = new_column.name();
         ColumnMapping* column_mapping = changer->get_mutable_column_mapping(i);
         column_mapping->new_column = &new_column;
 
@@ -1729,6 +1729,11 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
             continue;
         }
 
+        if (column_name.find("__doris_shadow_") == 0) {
+            // Should delete in the future, just a protection for bug.
+            LOG(INFO) << "a shadow column is encountered " << column_name;
+            return Status::InternalError("failed due to operate on shadow column");
+        }
         // Newly added column go here
         column_mapping->ref_column = -1;
 
@@ -1738,8 +1743,9 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
         RETURN_IF_ERROR(
                 _init_column_mapping(column_mapping, new_column, new_column.default_value()));
 
-        VLOG_TRACE << "A column with default value will be added after schema changing. "
-                   << "column=" << column_name << ", default_value=" << new_column.default_value();
+        LOG(INFO) << "A column with default value will be added after schema changing. "
+                  << "column=" << column_name << ", default_value=" << new_column.default_value()
+                  << " to table " << new_tablet->get_table_id();
     }
 
     if (materialized_function_map.count(WHERE_SIGN)) {
diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h
index 28be7c45a3..1bdd56b829 100644
--- a/be/src/olap/tablet.h
+++ b/be/src/olap/tablet.h
@@ -483,6 +483,8 @@ public:
         return config::max_tablet_io_errors > 0 && _io_error_times >= config::max_tablet_io_errors;
     }
 
+    int64_t get_table_id() { return _tablet_meta->table_id(); }
+
 private:
     Status _init_once_action();
     void _print_missed_versions(const std::vector<Version>& missed_versions) const;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
index 200a87b6da..35acb0272c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
@@ -698,6 +698,7 @@ public class SchemaChangeHandler extends AlterHandler {
              */
             modColumn.setName(SHADOW_NAME_PREFIX + modColumn.getName());
         }
+        LOG.info("modify column {} ", modColumn);
         return lightSchemaChange;
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
index d6a5b1b576..83427f1579 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
@@ -270,6 +270,10 @@ public class Column implements Writable, GsonPostProcessable {
         return this.name;
     }
 
+    public String getNonShadowName() {
+        return removeNamePrefix(name);
+    }
+
     public String getNameWithoutMvPrefix() {
         return CreateMaterializedViewStmt.mvColumnBreaker(name);
     }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java b/fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java
index 0ebd3fb7e5..a3f80b6c93 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java
@@ -221,7 +221,7 @@ public class OlapTableSink extends DataSink {
             List<String> columns = Lists.newArrayList();
             List<TColumn> columnsDesc = Lists.newArrayList();
             List<TOlapTableIndex> indexDesc = Lists.newArrayList();
-            columns.addAll(indexMeta.getSchema().stream().map(Column::getName).collect(Collectors.toList()));
+            columns.addAll(indexMeta.getSchema().stream().map(Column::getNonShadowName).collect(Collectors.toList()));
             for (Column column : indexMeta.getSchema()) {
                 TColumn tColumn = column.toThrift();
                 column.setIndexFlag(tColumn, table);
diff --git a/regression-test/suites/schema_change_p0/decimalv3/test_agg_keys_schema_change_decimalv3.groovy b/regression-test/suites/schema_change_p0/decimalv3/test_agg_keys_schema_change_decimalv3.groovy
index 321b1cd1a2..45f5be6701 100644
--- a/regression-test/suites/schema_change_p0/decimalv3/test_agg_keys_schema_change_decimalv3.groovy
+++ b/regression-test/suites/schema_change_p0/decimalv3/test_agg_keys_schema_change_decimalv3.groovy
@@ -21,6 +21,7 @@ suite("test_agg_keys_schema_change_decimalv3") {
     def tbName = "test_agg_keys_schema_change_decimalv3"
     def getJobState = { tableName ->
          def jobStateResult = sql """  SHOW ALTER TABLE COLUMN WHERE IndexName='${tableName}' ORDER BY createtime DESC LIMIT 1 """
+         logger.info(jobStateResult.toString());
          return jobStateResult[0][9]
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org