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/incubator-doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 123e139  [Delete] Allow delete duplicated non-key column using delete 
from  (#3424)
123e139 is described below

commit 123e1394b167247bd48adf04be6b104362f0c23a
Author: yangzhg <780531...@qq.com>
AuthorDate: Fri May 15 09:26:36 2020 +0800

    [Delete] Allow delete duplicated non-key column using delete from  (#3424)
---
 be/CMakeLists.txt                                  |  16 ++--
 be/src/olap/delete_handler.cpp                     |   5 +-
 be/src/olap/olap_cond.cpp                          |  12 +--
 be/src/olap/olap_cond.h                            |   4 +
 be/src/olap/rowset/alpha_rowset.cpp                |   2 +-
 be/src/olap/rowset/column_data.cpp                 |   6 +-
 be/src/olap/rowset/column_data.h                   |   3 +-
 be/src/olap/rowset/column_data_writer.cpp          |   4 +-
 be/src/olap/rowset/segment_group.cpp               |  17 +++-
 be/src/olap/rowset/segment_group.h                 |   4 +
 be/src/olap/rowset/segment_v2/segment_writer.cpp   |   2 +-
 be/src/olap/schema_change.cpp                      |   5 +-
 be/src/runtime/thread_resource_mgr.cpp             |  10 +-
 be/src/util/path_util.cpp                          |   3 +
 be/test/olap/delete_handler_test.cpp               | 104 +++++++++++++++++++++
 .../doris/alter/MaterializedViewHandler.java       |   8 +-
 .../java/org/apache/doris/load/DeleteHandler.java  |  17 +++-
 17 files changed, 177 insertions(+), 45 deletions(-)

diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index aa67c27..4f37075 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -293,7 +293,9 @@ set(CXX_GCC_FLAGS "${CXX_GCC_FLAGS} -g 
-Wno-unused-local-typedefs")
 # For CMAKE_BUILD_TYPE=Debug
 #   -ggdb: Enable gdb debugging
 # Debug information is stored as dwarf2 to be as compatible as possible
-set(CXX_FLAGS_DEBUG "${CXX_GCC_FLAGS} -ggdb -O0 -gdwarf-2")
+#   -Werror: compile warnings should be errors when using the toolchain 
compiler.
+# Only enable for debug builds because this is what we test in pre-commit 
tests.
+set(CXX_FLAGS_DEBUG "${CXX_GCC_FLAGS} -Werror -ggdb -O0 -gdwarf-2")
 
 # For CMAKE_BUILD_TYPE=Release
 #   -O3: Enable all compiler optimizations
@@ -301,24 +303,20 @@ set(CXX_FLAGS_DEBUG "${CXX_GCC_FLAGS} -ggdb -O0 
-gdwarf-2")
 #   -gdwarf-2: Debug information is stored as dwarf2 to be as compatible as 
possible
 set(CXX_FLAGS_RELEASE "${CXX_GCC_FLAGS} -O3 -gdwarf-2 -DNDEBUG")
 
-SET(CXX_FLAGS_ASAN "${CXX_GCC_FLAGS} -O1 -fsanitize=address 
-DADDRESS_SANITIZER")
-SET(CXX_FLAGS_LSAN "${CXX_GCC_FLAGS} -O1 -fsanitize=leak -DLEAK_SANITIZER")
+SET(CXX_FLAGS_ASAN "${CXX_GCC_FLAGS} -ggdb3 -O0 -gdwarf-2 -fsanitize=address 
-DADDRESS_SANITIZER")
+SET(CXX_FLAGS_LSAN "${CXX_GCC_FLAGS} -ggdb3 -O0 -gdwarf-2 -fsanitize=leak 
-DLEAK_SANITIZER")
 
 # Set the flags to the undefined behavior sanitizer, also known as "ubsan"
 # Turn on sanitizer and debug symbols to get stack traces:
-SET(CXX_FLAGS_UBSAN "${CXX_GCC_FLAGS} -ggdb3 -fsanitize=undefined")
+SET(CXX_FLAGS_UBSAN "${CXX_GCC_FLAGS} -ggdb3 -O0 -gdwarf-2 -fno-wrapv 
-fsanitize=undefined")
 # Ignore a number of noisy errors with too many false positives:
 # TODO(zc):
 # SET(CXX_FLAGS_UBSAN "${CXX_FLAGS_UBSAN} 
-fno-sanitize=alignment,function,vptr,float-divide-by-zero,float-cast-overflow")
 # Don't enforce wrapped signed integer arithmetic so that the sanitizer 
actually sees
-# undefined wrapping:
-SET(CXX_FLAGS_UBSAN "${CXX_FLAGS_UBSAN} -fno-wrapv")
-# To ease debugging, turn off all optimizations:
-SET(CXX_FLAGS_UBSAN "${CXX_FLAGS_UBSAN} -O0")
 
 # Set the flags to the thread sanitizer, also known as "tsan"
 # Turn on sanitizer and debug symbols to get stack traces:
-SET(CXX_FLAGS_TSAN "${CXX_GCC_FLAGS} -O1 -ggdb3 -fsanitize=thread 
-DTHREAD_SANITIZER")
+SET(CXX_FLAGS_TSAN "${CXX_GCC_FLAGS} -O0 -ggdb3 -fsanitize=thread 
-DTHREAD_SANITIZER")
 
 # Set compile flags based on the build type.
 if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
diff --git a/be/src/olap/delete_handler.cpp b/be/src/olap/delete_handler.cpp
index 5ee7e5c..3435dc3 100644
--- a/be/src/olap/delete_handler.cpp
+++ b/be/src/olap/delete_handler.cpp
@@ -105,10 +105,10 @@ OLAPStatus DeleteConditionHandler::check_condition_valid(
     // 检查指定的列是不是key,是不是float或doulbe类型
     const TabletColumn& column = schema.column(field_index);
 
-    if (!column.is_key()
+    if ((!column.is_key() && schema.keys_type() != KeysType::DUP_KEYS)
             || column.type() == OLAP_FIELD_TYPE_DOUBLE
             || column.type() == OLAP_FIELD_TYPE_FLOAT) {
-        LOG(WARNING) << "field is not key column, or its type is float or 
double.";
+        LOG(WARNING) << "field is not key column, or storage model is not 
duplicate, or data type is float or double.";
         return OLAP_ERR_DELETE_INVALID_CONDITION;
     }
 
@@ -352,4 +352,3 @@ void 
DeleteHandler::get_delete_conditions_after_version(int32_t version,
 }
 
 }  // namespace doris
-
diff --git a/be/src/olap/olap_cond.cpp b/be/src/olap/olap_cond.cpp
index 5707ef2..c0f4494 100644
--- a/be/src/olap/olap_cond.cpp
+++ b/be/src/olap/olap_cond.cpp
@@ -637,7 +637,7 @@ bool Conditions::delete_conditions_eval(const RowCursor& 
row) const {
     }
     
     for (auto& each_cond : _columns) {
-        if (each_cond.second->is_key() && !each_cond.second->eval(row)) {
+        if (_cond_column_is_key_or_duplicate(each_cond.second) && 
!each_cond.second->eval(row)) {
             return false;
         }
     }
@@ -651,13 +651,13 @@ bool Conditions::delete_conditions_eval(const RowCursor& 
row) const {
 bool Conditions::rowset_pruning_filter(const std::vector<KeyRange>& zone_maps) 
const {
     //通过所有列上的删除条件对version进行过滤
     for (auto& cond_it : _columns) {
-        if (cond_it.second->is_key() && cond_it.first > zone_maps.size()) {
+        if (_cond_column_is_key_or_duplicate(cond_it.second) && cond_it.first 
> zone_maps.size()) {
             LOG(WARNING) << "where condition not equal zone maps size. "
                          << "cond_id=" << cond_it.first
                          << ", zone_map_size=" << zone_maps.size();
             return false;
         }
-        if (cond_it.second->is_key() && 
!cond_it.second->eval(zone_maps[cond_it.first])) {
+        if (_cond_column_is_key_or_duplicate(cond_it.second) && 
!cond_it.second->eval(zone_maps.at(cond_it.first))) {
             return true;
         }
     }
@@ -681,9 +681,9 @@ int Conditions::delete_pruning_filter(const 
std::vector<KeyRange>& zone_maps) co
     for (auto& cond_it : _columns) {
         /*
          * this is base on the assumption that the delete condition
-         * is only about key field, not about value field.
+         * is only about key field, not about value field except the storage 
model is duplicate.
         */
-        if (cond_it.second->is_key() && cond_it.first > zone_maps.size()) {
+        if (_cond_column_is_key_or_duplicate(cond_it.second) && cond_it.first 
> zone_maps.size()) {
             LOG(WARNING) << "where condition not equal column statistics size. 
"
                          << "cond_id=" << cond_it.first
                          << ", zone_map_size=" << zone_maps.size();
@@ -691,7 +691,7 @@ int Conditions::delete_pruning_filter(const 
std::vector<KeyRange>& zone_maps) co
             continue;
         }
 
-        int del_ret = cond_it.second->del_eval(zone_maps[cond_it.first]);
+        int del_ret = cond_it.second->del_eval(zone_maps.at(cond_it.first));
         if (DEL_SATISFIED == del_ret) {
             continue;
         } else if (DEL_PARTIAL_SATISFIED == del_ret) {
diff --git a/be/src/olap/olap_cond.h b/be/src/olap/olap_cond.h
index a1987c4..7166101 100644
--- a/be/src/olap/olap_cond.h
+++ b/be/src/olap/olap_cond.h
@@ -193,6 +193,10 @@ private:
         return -1;
     }
 
+    bool _cond_column_is_key_or_duplicate(const CondColumn* cc) const {
+        return cc->is_key() || _schema->keys_type() == KeysType::DUP_KEYS;
+    }
+
 private:
     const TabletSchema* _schema;
     CondColumns _columns;   // list of condition column
diff --git a/be/src/olap/rowset/alpha_rowset.cpp 
b/be/src/olap/rowset/alpha_rowset.cpp
index e1a20f7..7b17f48 100644
--- a/be/src/olap/rowset/alpha_rowset.cpp
+++ b/be/src/olap/rowset/alpha_rowset.cpp
@@ -300,7 +300,7 @@ OLAPStatus AlphaRowset::init() {
 
         if (segment_group_meta.zone_maps_size() != 0) {
             size_t zone_maps_size = segment_group_meta.zone_maps_size();
-            size_t num_key_columns = _schema->num_key_columns();
+            size_t num_key_columns = _schema->keys_type() == 
KeysType::DUP_KEYS ? _schema->num_columns() : _schema->num_key_columns();
             if (num_key_columns != zone_maps_size) {
                 LOG(ERROR) << "column pruning size is error."
                         << "zone_maps_size=" << zone_maps_size << ", "
diff --git a/be/src/olap/rowset/column_data.cpp 
b/be/src/olap/rowset/column_data.cpp
index 0bc9609..080564d 100644
--- a/be/src/olap/rowset/column_data.cpp
+++ b/be/src/olap/rowset/column_data.cpp
@@ -36,6 +36,7 @@ ColumnData::ColumnData(SegmentGroup* segment_group)
         _col_predicates(nullptr),
         _delete_status(DEL_NOT_SATISFIED),
         _runtime_state(nullptr),
+        _schema(segment_group->get_tablet_schema()),
         _is_using_cache(false),
         _segment_reader(nullptr),
         _lru_cache(nullptr) {
@@ -509,7 +510,10 @@ int ColumnData::delete_pruning_filter() {
         return DEL_NOT_SATISFIED;
     }
 
-    if (!_segment_group->has_zone_maps()) {
+    int num_zone_maps = _schema.keys_type() == KeysType::DUP_KEYS ? 
_schema.num_columns() : _schema.num_key_columns();
+    // _segment_group->get_zone_maps().size() < num_zone_maps for a table is 
schema changed from older version that not support
+    // generate zone map for duplicated mode value column, using 
DEL_PARTIAL_SATISFIED
+    if (!_segment_group->has_zone_maps() || 
_segment_group->get_zone_maps().size() < num_zone_maps)  {
         /*
          * if segment_group has no column statistics, we cannot judge whether 
the data can be filtered or not
          */
diff --git a/be/src/olap/rowset/column_data.h b/be/src/olap/rowset/column_data.h
index 1e1c822..06b9a8c 100644
--- a/be/src/olap/rowset/column_data.h
+++ b/be/src/olap/rowset/column_data.h
@@ -113,7 +113,6 @@ public:
     SegmentGroup* segment_group() const { return _segment_group; }
     void set_segment_group(SegmentGroup* segment_group) { _segment_group = 
segment_group; }
     int64_t num_rows() const { return _segment_group->num_rows(); }
-    Tablet* tablet() const { return _tablet; }
 
     // To compatable with schmea change read, use this function to init column 
data
     // for schema change read. Only called in get_first_row_block
@@ -165,7 +164,7 @@ private:
     RuntimeState* _runtime_state;
     OlapReaderStatistics* _stats;
 
-    Tablet* _tablet;
+    const TabletSchema& _schema;
     // whether in normal read, use return columns to load block
     bool _is_normal_read = false;
     bool _end_key_is_set = false;
diff --git a/be/src/olap/rowset/column_data_writer.cpp 
b/be/src/olap/rowset/column_data_writer.cpp
index 8900d41..2a501e4 100644
--- a/be/src/olap/rowset/column_data_writer.cpp
+++ b/be/src/olap/rowset/column_data_writer.cpp
@@ -40,7 +40,7 @@ ColumnDataWriter::ColumnDataWriter(SegmentGroup* 
segment_group,
       _is_push_write(is_push_write),
       _compress_kind(compress_kind),
       _bloom_filter_fpp(bloom_filter_fpp),
-      _zone_maps(segment_group->get_num_key_columns(), KeyRange(NULL, NULL)),
+      _zone_maps(segment_group->get_num_zone_map_columns(), KeyRange(NULL, 
NULL)),
       _row_index(0),
       _row_block(NULL),
       _segment_writer(NULL),
@@ -140,7 +140,7 @@ OLAPStatus ColumnDataWriter::write(const RowType& row) {
 
 template<typename RowType>
 void ColumnDataWriter::next(const RowType& row) {
-    for (size_t cid = 0; cid < _segment_group->get_num_key_columns(); ++cid) {
+    for (size_t cid = 0; cid < _segment_group->get_num_zone_map_columns(); 
++cid) {
         auto field = row.schema()->column(cid);
         auto cell = row.cell(cid);
 
diff --git a/be/src/olap/rowset/segment_group.cpp 
b/be/src/olap/rowset/segment_group.cpp
index 64ba139..da528aa 100644
--- a/be/src/olap/rowset/segment_group.cpp
+++ b/be/src/olap/rowset/segment_group.cpp
@@ -249,13 +249,13 @@ OLAPStatus 
SegmentGroup::add_zone_maps_for_linked_schema_change(
         return OLAP_SUCCESS;
     }
 
-    // 1. rollup tablet num_key_columns() will less than base tablet 
zone_map_fields.size().
+    // 1. rollup tablet get_num_zone_map_columns() will less than base tablet 
zone_map_fields.size().
     //    For LinkedSchemaChange, the rollup tablet keys order is the same as 
base tablet
-    // 2. adding column to existed table, num_key_columns() will larger than
+    // 2. adding column to existed table, get_num_zone_map_columns() will 
larger than
     //    zone_map_fields.size()
 
     int num_new_keys = 0;
-    for (size_t i = 0; i < _schema->num_key_columns(); ++i) {
+    for (size_t i = 0; i < get_num_zone_map_columns(); ++i) {
         const TabletColumn& column = _schema->column(i);
 
         WrapperField* first = WrapperField::create(column);
@@ -283,7 +283,7 @@ OLAPStatus 
SegmentGroup::add_zone_maps_for_linked_schema_change(
 
 OLAPStatus SegmentGroup::add_zone_maps(
         const std::vector<std::pair<WrapperField*, WrapperField*>>& 
zone_map_fields) {
-    DCHECK(_empty || zone_map_fields.size() == _schema->num_key_columns());
+    DCHECK(_empty || zone_map_fields.size() == get_num_zone_map_columns());
     for (size_t i = 0; i < zone_map_fields.size(); ++i) {
         const TabletColumn& column = _schema->column(i);
         WrapperField* first = WrapperField::create(column);
@@ -302,7 +302,7 @@ OLAPStatus SegmentGroup::add_zone_maps(
 OLAPStatus SegmentGroup::add_zone_maps(
         std::vector<std::pair<std::string, std::string> > &zone_map_strings,
         std::vector<bool> &null_vec) {
-    DCHECK(_empty || zone_map_strings.size() == _schema->num_key_columns());
+    DCHECK(_empty || zone_map_strings.size() <= get_num_zone_map_columns());
     for (size_t i = 0; i < zone_map_strings.size(); ++i) {
         const TabletColumn& column = _schema->column(i);
         WrapperField* first = WrapperField::create(column);
@@ -697,6 +697,13 @@ const TabletSchema& SegmentGroup::get_tablet_schema() {
     return *_schema;
 }
 
+int SegmentGroup::get_num_zone_map_columns() {
+    if (_schema->keys_type() == KeysType::DUP_KEYS) {
+        return _schema->num_columns();
+    }
+    return _schema->num_key_columns();
+}
+
 int SegmentGroup::get_num_key_columns() {
     return _schema->num_key_columns();
 }
diff --git a/be/src/olap/rowset/segment_group.h 
b/be/src/olap/rowset/segment_group.h
index 09597be..7bf8523 100644
--- a/be/src/olap/rowset/segment_group.h
+++ b/be/src/olap/rowset/segment_group.h
@@ -239,6 +239,10 @@ public:
 
     int get_num_key_columns();
 
+    // for agg, uniq and replace model  get_num_zone_map_columns() = 
get_num_key_columns(),
+    // for duplicate mode get_num_zone_map_columns() == num_columns
+    int get_num_zone_map_columns();
+
     int get_num_short_key_columns();
 
     size_t get_num_rows_per_row_block();
diff --git a/be/src/olap/rowset/segment_v2/segment_writer.cpp 
b/be/src/olap/rowset/segment_v2/segment_writer.cpp
index eb89bec..a977407 100644
--- a/be/src/olap/rowset/segment_v2/segment_writer.cpp
+++ b/be/src/olap/rowset/segment_v2/segment_writer.cpp
@@ -67,7 +67,7 @@ Status SegmentWriter::init(uint32_t write_mbytes_per_sec 
__attribute__((unused))
         opts.meta->set_is_nullable(column.is_nullable());
 
         // now we create zone map for key columns
-        opts.need_zone_map = column.is_key();
+        opts.need_zone_map = column.is_key() || _tablet_schema->keys_type() == 
KeysType::DUP_KEYS;
         opts.need_bloom_filter = column.is_bf_column();
         opts.need_bitmap_index = column.has_bitmap_index();
 
diff --git a/be/src/olap/schema_change.cpp b/be/src/olap/schema_change.cpp
index 65725b7..690fe89 100644
--- a/be/src/olap/schema_change.cpp
+++ b/be/src/olap/schema_change.cpp
@@ -265,12 +265,11 @@ bool RowBlockChanger::change_row_block(
     for (size_t i = 0, len = mutable_block->tablet_schema().num_columns(); 
!filter_all && i < len; ++i) {
         int32_t ref_column = _schema_mapping[i].ref_column;
 
-        FieldType reftype = 
ref_block->tablet_schema().column(ref_column).type();
-        FieldType newtype = mutable_block->tablet_schema().column(i).type();
-
         if (_schema_mapping[i].ref_column >= 0) {
             // new column will be assigned as referenced column
             // check if the type of new column is equal to the older's.
+            FieldType reftype = 
ref_block->tablet_schema().column(ref_column).type();
+            FieldType newtype = 
mutable_block->tablet_schema().column(i).type();
             if (newtype == reftype) {
                 // 效率低下,也可以直接计算变长域拷贝,但仍然会破坏封装
                 for (size_t row_index = 0, new_row_index = 0;
diff --git a/be/src/runtime/thread_resource_mgr.cpp 
b/be/src/runtime/thread_resource_mgr.cpp
index 30f2566..4cc8627 100644
--- a/be/src/runtime/thread_resource_mgr.cpp
+++ b/be/src/runtime/thread_resource_mgr.cpp
@@ -93,10 +93,12 @@ ThreadResourceMgr::ResourcePool* 
ThreadResourceMgr::register_pool() {
 void ThreadResourceMgr::unregister_pool(ResourcePool* pool) {
     DCHECK(pool != NULL);
     boost::unique_lock< boost::mutex> l(_lock);
-    DCHECK(_pools.find(pool) != _pools.end());
-    _pools.erase(pool);
-    _free_pool_objs.push_back(pool);
-    update_pool_quotas();
+    // this may be double unregisted after pr #3326 by LaiYingChun, so check 
if the pool is already unregisted
+    if (_pools.find(pool) != _pools.end()) {
+        _pools.erase(pool);
+        _free_pool_objs.push_back(pool);
+        update_pool_quotas();
+    }
 }
 
 void 
ThreadResourceMgr::ResourcePool::set_thread_available_cb(thread_available_cb 
fn) {
diff --git a/be/src/util/path_util.cpp b/be/src/util/path_util.cpp
index 5f38f1c..5252618 100644
--- a/be/src/util/path_util.cpp
+++ b/be/src/util/path_util.cpp
@@ -70,6 +70,9 @@ vector<string> split_path(const string& path) {
     return segments;
 }
 
+// strdup use malloc to obtain memory for the new string, it should be freed 
with free.
+// but unique_ptr use delete to free memory by default, so it should specify 
free memory using free
+
 string dir_name(const string& path) {
     std::vector<char> path_copy(path.c_str(), path.c_str() + path.size() + 1);
     return dirname(&path_copy[0]);
diff --git a/be/test/olap/delete_handler_test.cpp 
b/be/test/olap/delete_handler_test.cpp
index 4940b98..1ee19ca 100644
--- a/be/test/olap/delete_handler_test.cpp
+++ b/be/test/olap/delete_handler_test.cpp
@@ -153,6 +153,87 @@ void set_default_create_tablet_request(TCreateTabletReq* 
request) {
     request->tablet_schema.columns.push_back(v);
 }
 
+void set_create_duplicate_tablet_request(TCreateTabletReq* request) {
+    request->tablet_id = 10009;
+    request->__set_version(1);
+    request->__set_version_hash(0);
+    request->tablet_schema.schema_hash = 270068376;
+    request->tablet_schema.short_key_column_count = 2;
+    request->tablet_schema.keys_type = TKeysType::DUP_KEYS;
+    request->tablet_schema.storage_type = TStorageType::COLUMN;
+
+    TColumn k1;
+    k1.column_name = "k1";
+    k1.__set_is_key(true);
+    k1.column_type.type = TPrimitiveType::TINYINT;
+    request->tablet_schema.columns.push_back(k1);
+
+    TColumn k2;
+    k2.column_name = "k2";
+    k2.__set_is_key(true);
+    k2.column_type.type = TPrimitiveType::SMALLINT;
+    request->tablet_schema.columns.push_back(k2);
+
+    TColumn k3;
+    k3.column_name = "k3";
+    k3.__set_is_key(true);
+    k3.column_type.type = TPrimitiveType::INT;
+    request->tablet_schema.columns.push_back(k3);
+
+    TColumn k4;
+    k4.column_name = "k4";
+    k4.__set_is_key(true);
+    k4.column_type.type = TPrimitiveType::BIGINT;
+    request->tablet_schema.columns.push_back(k4);
+
+    TColumn k5;
+    k5.column_name = "k5";
+    k5.__set_is_key(true);
+    k5.column_type.type = TPrimitiveType::LARGEINT;
+    request->tablet_schema.columns.push_back(k5);
+
+    TColumn k9;
+    k9.column_name = "k9";
+    k9.__set_is_key(true);
+    k9.column_type.type = TPrimitiveType::DECIMAL;
+    k9.column_type.__set_precision(6);
+    k9.column_type.__set_scale(3);
+    request->tablet_schema.columns.push_back(k9);
+
+    TColumn k10;
+    k10.column_name = "k10";
+    k10.__set_is_key(true);
+    k10.column_type.type = TPrimitiveType::DATE;
+    request->tablet_schema.columns.push_back(k10);
+
+    TColumn k11;
+    k11.column_name = "k11";
+    k11.__set_is_key(true);
+    k11.column_type.type = TPrimitiveType::DATETIME;
+    request->tablet_schema.columns.push_back(k11);
+
+    TColumn k12;
+    k12.column_name = "k12";
+    k12.__set_is_key(true);
+    k12.column_type.__set_len(64);
+    k12.column_type.type = TPrimitiveType::CHAR;
+    request->tablet_schema.columns.push_back(k12);
+
+    TColumn k13;
+    k13.column_name = "k13";
+    k13.__set_is_key(true);
+    k13.column_type.__set_len(64);
+    k13.column_type.type = TPrimitiveType::VARCHAR;
+    request->tablet_schema.columns.push_back(k13);
+
+    TColumn v;
+    v.column_name = "v";
+    v.__set_is_key(false);
+    v.column_type.type = TPrimitiveType::BIGINT;
+    request->tablet_schema.columns.push_back(v);
+}
+
+
 void set_default_push_request(TPushReq* request) {
     request->tablet_id = 10003;
     request->schema_hash = 270068375;
@@ -180,19 +261,31 @@ protected:
                 _create_tablet.tablet_id, 
_create_tablet.tablet_schema.schema_hash);
         ASSERT_TRUE(tablet.get() != NULL);
         _tablet_path = tablet->tablet_path();
+
+        set_create_duplicate_tablet_request(&_create_dup_tablet);
+        res = k_engine->create_tablet(_create_dup_tablet);
+        ASSERT_EQ(OLAP_SUCCESS, res);
+        dup_tablet = k_engine->tablet_manager()->get_tablet(
+                _create_dup_tablet.tablet_id, 
_create_dup_tablet.tablet_schema.schema_hash);
+        ASSERT_TRUE(dup_tablet.get() != NULL);
+        _dup_tablet_path = tablet->tablet_path();
     }
 
     void TearDown() {
         // Remove all dir.
         tablet.reset();
+        dup_tablet.reset();
         StorageEngine::instance()->tablet_manager()->drop_tablet(
                 _create_tablet.tablet_id, 
_create_tablet.tablet_schema.schema_hash);
         ASSERT_TRUE(FileUtils::remove_all(config::storage_root_path).ok());
     }
 
     std::string _tablet_path;
+    std::string _dup_tablet_path;
     TabletSharedPtr tablet;
+    TabletSharedPtr dup_tablet;
     TCreateTabletReq _create_tablet;
+    TCreateTabletReq _create_dup_tablet;
     DeleteConditionHandler _delete_condition_handler;
 };
 
@@ -263,6 +356,17 @@ TEST_F(TestDeleteConditionHandler, 
StoreCondNonexistentColumn) {
 
     failed_res = 
_delete_condition_handler.generate_delete_predicate(tablet->tablet_schema(), 
conditions, &del_pred);;
     ASSERT_EQ(OLAP_ERR_DELETE_INVALID_CONDITION, failed_res);
+
+    // value column in duplicate model can be deleted;
+    conditions.clear();
+    condition.column_name = "v";
+    condition.condition_op = "=";
+    condition.condition_values.clear();
+    condition.condition_values.push_back("5");
+    conditions.push_back(condition);
+
+    OLAPStatus success_res = 
_delete_condition_handler.generate_delete_predicate(dup_tablet->tablet_schema(),
 conditions, &del_pred);;
+    ASSERT_EQ(OLAP_SUCCESS, success_res);
 }
 
 // 测试删除条件值不符合类型要求
diff --git 
a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java 
b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
index 1f1b90b..0d3c792 100644
--- a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
+++ b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
@@ -231,7 +231,6 @@ public class MaterializedViewHandler extends AlterHandler {
 
                 // step 1 check whether current alter is change storage format
                 String rollupIndexName = addRollupClause.getRollupName();
-                String newStorageFormatIndexName = "__v2_" + 
olapTable.getName();
                 boolean changeStorageFormat = false;
                 if (rollupIndexName.equalsIgnoreCase(olapTable.getName())) {
                     // for upgrade test to create segment v2 rollup index by 
using the sql:
@@ -242,7 +241,7 @@ public class MaterializedViewHandler extends AlterHandler {
                         throw new DdlException("Table[" + olapTable.getName() 
+ "] can not " +
                                 "add segment v2 rollup index without setting 
storage format to v2.");
                     }
-                    rollupIndexName = newStorageFormatIndexName;
+                    rollupIndexName = NEW_STORAGE_FORMAT_INDEX_NAME_PREFIX + 
olapTable.getName();
                     changeStorageFormat = true;
                 }
 
@@ -549,7 +548,10 @@ public class MaterializedViewHandler extends AlterHandler {
                     }
                     keyStorageLayoutBytes += 
baseColumn.getType().getStorageLayoutBytes();
                     Column rollupColumn = new Column(baseColumn);
-                    if ((i + 1) <= FeConstants.shortkey_max_column_count
+                    if(changeStorageFormat) {
+                        rollupColumn.setIsKey(baseColumn.isKey());
+                        
rollupColumn.setAggregationType(baseColumn.getAggregationType(), true);
+                    } else if ((i + 1) <= FeConstants.shortkey_max_column_count
                             || keyStorageLayoutBytes < 
FeConstants.shortkey_maxsize_bytes) {
                         rollupColumn.setIsKey(true);
                         rollupColumn.setAggregationType(null, false);
diff --git a/fe/src/main/java/org/apache/doris/load/DeleteHandler.java 
b/fe/src/main/java/org/apache/doris/load/DeleteHandler.java
index 25ea614..b503937 100644
--- a/fe/src/main/java/org/apache/doris/load/DeleteHandler.java
+++ b/fe/src/main/java/org/apache/doris/load/DeleteHandler.java
@@ -29,6 +29,7 @@ import org.apache.doris.catalog.Database;
 import org.apache.doris.catalog.KeysType;
 import org.apache.doris.catalog.MaterializedIndex;
 import org.apache.doris.catalog.MaterializedIndex.IndexExtState;
+import org.apache.doris.catalog.MaterializedIndexMeta;
 import org.apache.doris.catalog.OlapTable;
 import org.apache.doris.catalog.Partition;
 import org.apache.doris.catalog.PartitionType;
@@ -478,9 +479,14 @@ public class DeleteHandler implements Writable {
             }
 
             Column column = nameToColumn.get(columnName);
-            if (!column.isKey()) {
+            // Due to rounding errors, most floating-point numbers end up 
being slightly imprecise,
+            // it also means that numbers expected to be equal often differ 
slightly, so we do not allow compare with
+            // floating-point numbers, floating-point number not allowed in 
where clause
+            if (!column.isKey() && table.getKeysType() != KeysType.DUP_KEYS
+                    || column.getDataType().isFloatingPointType()) {
                 // 
ErrorReport.reportDdlException(ErrorCode.ERR_NOT_KEY_COLUMN, columnName);
-                throw new DdlException("Column[" + columnName + "] is not key 
column");
+                throw new DdlException("Column[" + columnName + "] is not key 
column or storage model " +
+                        "is not duplicate or column type is float or double.");
             }
 
             if (condition instanceof BinaryPredicate) {
@@ -517,10 +523,11 @@ public class DeleteHandler implements Writable {
                 }
                 Column column = indexColNameToColumn.get(columnName);
                 if (column == null) {
-                    
ErrorReport.reportDdlException(ErrorCode.ERR_BAD_FIELD_ERROR, columnName, 
indexName);
+                    
ErrorReport.reportDdlException(ErrorCode.ERR_BAD_FIELD_ERROR, columnName,
+                            table.getBaseIndexId() == index.getId()? indexName 
: "index[" + indexName +"]");
                 }
-
-                if (table.getKeysType() == KeysType.DUP_KEYS && 
!column.isKey()) {
+                MaterializedIndexMeta indexMeta = 
table.getIndexIdToMeta().get(index.getId());
+                if (indexMeta.getKeysType() != KeysType.DUP_KEYS && 
!column.isKey()) {
                     throw new DdlException("Column[" + columnName + "] is not 
key column in index[" + indexName + "]");
                 }
             }


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

Reply via email to