From 43f3cfafd38fd5163fea3cf29bbc837d80e1fd6b Mon Sep 17 00:00:00 2001 From: amorynan Date: Mon, 1 Apr 2024 20:53:47 +0800 Subject: [PATCH 1/5] fix vertical_compaction_reader for agg table with array/map type --- be/src/vec/olap/vertical_block_reader.cpp | 22 ++- be/src/vec/olap/vertical_block_reader.h | 2 +- ...est_compaction_agg_keys_with_array_map.out | 13 ++ ..._compaction_agg_keys_with_array_map.groovy | 154 ++++++++++++++++++ 4 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 regression-test/data/compaction/test_compaction_agg_keys_with_array_map.out create mode 100644 regression-test/suites/compaction/test_compaction_agg_keys_with_array_map.groovy diff --git a/be/src/vec/olap/vertical_block_reader.cpp b/be/src/vec/olap/vertical_block_reader.cpp index 3fc3d52f9bcdad..74ac6d3805f149 100644 --- a/be/src/vec/olap/vertical_block_reader.cpp +++ b/be/src/vec/olap/vertical_block_reader.cpp @@ -181,7 +181,7 @@ void VerticalBlockReader::_init_agg_state(const ReaderParams& read_params) { _next_row.block->create_same_struct_block(_reader_context.batch_size)->mutate_columns(); _stored_has_null_tag.resize(_stored_data_columns.size()); - _stored_has_string_tag.resize(_stored_data_columns.size()); + _stored_has_variable_length_tag.resize(_stored_data_columns.size()); auto& tablet_schema = *_tablet_schema; for (size_t idx = 0; idx < _return_columns.size(); ++idx) { @@ -198,13 +198,23 @@ void VerticalBlockReader::_init_agg_state(const ReaderParams& read_params) { }); _agg_places.push_back(place); - // calculate `has_string` tag. - _stored_has_string_tag[idx] = + // calculate `_has_variable_length_tag` tag. like string, array, map + _stored_has_variable_length_tag[idx] = _stored_data_columns[idx]->is_column_string() || (_stored_data_columns[idx]->is_nullable() && reinterpret_cast(_stored_data_columns[idx].get()) ->get_nested_column_ptr() - ->is_column_string()); + ->is_column_string()) || + _stored_data_columns[idx]->is_column_array() || + (_stored_data_columns[idx]->is_nullable() && + reinterpret_cast(_stored_data_columns[idx].get()) + ->get_nested_column_ptr() + ->is_column_array()) || + _stored_data_columns[idx]->is_column_map() || + (_stored_data_columns[idx]->is_nullable() && + reinterpret_cast(_stored_data_columns[idx].get()) + ->get_nested_column_ptr() + ->is_column_map()); } } @@ -333,8 +343,8 @@ size_t VerticalBlockReader::_copy_agg_data() { } for (size_t idx = 0; idx < _return_columns.size(); ++idx) { auto& dst_column = _stored_data_columns[idx]; - if (_stored_has_string_tag[idx]) { - //string type should replace ordered + if (_stored_has_variable_length_tag[idx]) { + //variable length type should replace ordered for (size_t i = 0; i < copy_size; i++) { auto& ref = _stored_row_ref[i]; dst_column->replace_column_data(*ref.block->get_by_position(idx).column, diff --git a/be/src/vec/olap/vertical_block_reader.h b/be/src/vec/olap/vertical_block_reader.h index 2c65fd616b1957..77a01587b5873a 100644 --- a/be/src/vec/olap/vertical_block_reader.h +++ b/be/src/vec/olap/vertical_block_reader.h @@ -119,7 +119,7 @@ class VerticalBlockReader final : public TabletReader { std::vector _stored_row_ref; std::vector _stored_has_null_tag; - std::vector _stored_has_string_tag; + std::vector _stored_has_variable_length_tag; phmap::flat_hash_map>> _temp_ref_map; diff --git a/regression-test/data/compaction/test_compaction_agg_keys_with_array_map.out b/regression-test/data/compaction/test_compaction_agg_keys_with_array_map.out new file mode 100644 index 00000000000000..5c5ee3ed329b1d --- /dev/null +++ b/regression-test/data/compaction/test_compaction_agg_keys_with_array_map.out @@ -0,0 +1,13 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !select_default -- +1 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 ["a", "b", "c"] {"b":1, "c":2} +2 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 ["amory", "doris", "2024-04-29"] {"c":2} +3 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 \N \N +4 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 [null, "sdf"] \N + +-- !select_default2 -- +1 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 ["a", "b", "c"] {"b":1, "c":2} +2 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 ["amory", "doris", "2024-04-29"] {"c":2} +3 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 \N \N +4 2017-10-01 2017-10-01 2017-10-01T11:11:11.110 2017-10-01T11:11:11.110111 Beijing 10 1 [null, "sdf"] \N + diff --git a/regression-test/suites/compaction/test_compaction_agg_keys_with_array_map.groovy b/regression-test/suites/compaction/test_compaction_agg_keys_with_array_map.groovy new file mode 100644 index 00000000000000..1556d2f00a506f --- /dev/null +++ b/regression-test/suites/compaction/test_compaction_agg_keys_with_array_map.groovy @@ -0,0 +1,154 @@ +// 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("test_compaction_agg_keys_with_array_map") { + def tableName = "compaction_agg_keys_regression_test_complex" + + try { + String backend_id; + def backendId_to_backendIP = [:] + def backendId_to_backendHttpPort = [:] + getBackendIpHttpPort(backendId_to_backendIP, backendId_to_backendHttpPort); + backend_id = backendId_to_backendIP.keySet()[0] + + def (code, out, err) = show_be_config(backendId_to_backendIP.get(backend_id), backendId_to_backendHttpPort.get(backend_id)) + logger.info("Show config: code=" + code + ", out=" + out + ", err=" + err) + assertEquals(code, 0) + def configList = parseJson(out.trim()) + assert configList instanceof List + + boolean disableAutoCompaction = true + for (Object ele in (List) configList) { + assert ele instanceof List + if (((List) ele)[0] == "disable_auto_compaction") { + disableAutoCompaction = Boolean.parseBoolean(((List) ele)[2]) + } + } + + sql """ DROP TABLE IF EXISTS ${tableName} """ + sql """ + CREATE TABLE IF NOT EXISTS ${tableName} ( + `user_id` LARGEINT NOT NULL COMMENT "用户id", + `date` DATE NOT NULL COMMENT "数据灌入日期时间", + `datev2` DATEV2 NOT NULL COMMENT "数据灌入日期时间", + `datetimev2_1` DATETIMEV2(3) NOT NULL COMMENT "数据灌入日期时间", + `datetimev2_2` DATETIMEV2(6) NOT NULL COMMENT "数据灌入日期时间", + `city` VARCHAR(20) COMMENT "用户所在城市", + `age` SMALLINT COMMENT "用户年龄", + `sex` TINYINT COMMENT "用户性别", + `array_col` ARRAY REPLACE NULL COMMENT "array column", + `map_col` MAP REPLACE NULL COMMENT "map column") + AGGREGATE KEY(`user_id`, `date`, `datev2`, `datetimev2_1`, `datetimev2_2`, `city`, `age`, `sex`) DISTRIBUTED BY HASH(`user_id`) + PROPERTIES ( "replication_num" = "1" ); + """ + + sql """ INSERT INTO ${tableName} VALUES + (1, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, ['a', 'b'], map('a', 1)); + """ + + sql """ INSERT INTO ${tableName} VALUES + (1, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, ['a', 'b', 'c'], map('b', 1, 'c', 2)); + """ + + sql """ INSERT INTO ${tableName} VALUES + (2, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, ['amory', 'doris', 'commiter'], map('b', 1)); + """ + + sql """ INSERT INTO ${tableName} VALUES + (2, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, ['amory', 'doris', '2024-04-29'], map('c', 2)); + """ + + sql """ INSERT INTO ${tableName} VALUES + (3, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, ['e', 'f', 'g', 'd'], map()); + """ + + sql """ INSERT INTO ${tableName} VALUES + (3, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, ['e', 'f', 'g', 'd'], map('a', 1, 'b', 2)); + """ + + sql """ INSERT INTO ${tableName} VALUES + (3, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, NULL, NULL); + """ + + sql """ INSERT INTO ${tableName} VALUES + (4, '2017-10-01', '2017-10-01', '2017-10-01 11:11:11.110000', '2017-10-01 11:11:11.110111', 'Beijing', 10, 1, [NULL, 'sdf'], NULL); + """ + + qt_select_default """ SELECT * FROM ${tableName} t ORDER BY user_id; """ + + //TabletId,ReplicaId,BackendId,SchemaHash,Version,LstSuccessVersion,LstFailedVersion,LstFailedTime,LocalDataSize,RemoteDataSize,RowCount,State,LstConsistencyCheckTime,CheckVersion,QueryHits,VersionCount,PathHash,MetaUrl,CompactionStatus + def tablets = sql_return_maparray """ show tablets from ${tableName}; """ + + // trigger compactions for all tablets in ${tableName} + for (def tablet in tablets) { + String tablet_id = tablet.TabletId + backend_id = tablet.BackendId + (code, out, err) = be_run_cumulative_compaction(backendId_to_backendIP.get(backend_id), backendId_to_backendHttpPort.get(backend_id), tablet_id) + logger.info("Run compaction: code=" + code + ", out=" + out + ", err=" + err) + assertEquals(code, 0) + def compactJson = parseJson(out.trim()) + if (compactJson.status.toLowerCase() == "fail") { + assertEquals(disableAutoCompaction, false) + logger.info("Compaction was done automatically!") + } + if (disableAutoCompaction) { + assertEquals("success", compactJson.status.toLowerCase()) + } + } + + // wait for all compactions done + for (def tablet in tablets) { + boolean running = true + do { + Thread.sleep(1000) + String tablet_id = tablet.TabletId + backend_id = tablet.BackendId + (code, out, err) = be_get_compaction_status(backendId_to_backendIP.get(backend_id), backendId_to_backendHttpPort.get(backend_id), tablet_id) + logger.info("Get compaction status: code=" + code + ", out=" + out + ", err=" + err) + assertEquals(code, 0) + + def compactionStatus = parseJson(out.trim()) + assertEquals("success", compactionStatus.status.toLowerCase()) + running = compactionStatus.run_status + } while (running) + } + + def replicaNum = get_table_replica_num(tableName) + logger.info("get table replica num: " + replicaNum) + int rowCount = 0 + for (def tablet in tablets) { + String tablet_id = tablet.TabletId + + (code, out, err) = curl("GET", tablet.CompactionStatus) + logger.info("Show tablets status: code=" + code + ", out=" + out + ", err=" + err) + + assertEquals(code, 0) + def tabletJson = parseJson(out.trim()) + assert tabletJson.rowsets instanceof List + for (String rowset in (List) tabletJson.rowsets) { + rowCount += Integer.parseInt(rowset.split(" ")[1]) + } + } + assert (rowCount < 8 * replicaNum) + qt_select_default2 """ SELECT * FROM ${tableName} t ORDER BY user_id; """ + } finally { + try_sql("DROP TABLE IF EXISTS ${tableName}") + } +} + From e9a929c7409db66a0c069864f089553da7a4b9fc Mon Sep 17 00:00:00 2001 From: amorynan Date: Mon, 1 Apr 2024 20:57:22 +0800 Subject: [PATCH 2/5] update DCHCEK --- be/src/vec/columns/column_array.h | 2 +- be/src/vec/columns/column_string.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/be/src/vec/columns/column_array.h b/be/src/vec/columns/column_array.h index 17408bfa63311e..c6db17d2e01d0d 100644 --- a/be/src/vec/columns/column_array.h +++ b/be/src/vec/columns/column_array.h @@ -219,7 +219,7 @@ class ColumnArray final : public COWHelper { const uint32_t* indices_end) override; void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(size() > self_row); + DCHECK_EQ(size(), self_row + 1); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_string.h b/be/src/vec/columns/column_string.h index 9bf43c9c627815..f461ff887d2dc5 100644 --- a/be/src/vec/columns/column_string.h +++ b/be/src/vec/columns/column_string.h @@ -548,7 +548,7 @@ class ColumnString final : public COWHelper { } void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(size() > self_row); + DCHECK_EQ(size(), self_row + 1); const auto& r = assert_cast(rhs); auto data = r.get_data_at(row); From dc27c9c1e23f9302d68ec49d1f88d3afa379b7cd Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 2 Apr 2024 11:49:31 +0800 Subject: [PATCH 3/5] fix dcheck and normalize function --- be/src/vec/columns/column.h | 3 +++ be/src/vec/columns/column_array.h | 3 ++- be/src/vec/columns/column_const.h | 2 ++ be/src/vec/columns/column_map.h | 3 ++- be/src/vec/columns/column_nullable.h | 2 +- be/src/vec/columns/column_string.h | 10 ++++++++-- be/src/vec/olap/block_reader.cpp | 17 +---------------- be/src/vec/olap/vertical_block_reader.cpp | 17 +---------------- 8 files changed, 20 insertions(+), 37 deletions(-) diff --git a/be/src/vec/columns/column.h b/be/src/vec/columns/column.h index bf869961544817..f6749832363c50 100644 --- a/be/src/vec/columns/column.h +++ b/be/src/vec/columns/column.h @@ -630,6 +630,9 @@ class IColumn : public COW { /// Implies is_fixed_and_contiguous. virtual bool is_numeric() const { return false; } + // Column is ColumnString/ColumnArray/ColumnMap or other variable length column at every row + virtual bool is_variable_length() const { return false; } + virtual bool is_column_string() const { return false; } virtual bool is_column_decimal() const { return false; } diff --git a/be/src/vec/columns/column_array.h b/be/src/vec/columns/column_array.h index c6db17d2e01d0d..08fbb03ce716d8 100644 --- a/be/src/vec/columns/column_array.h +++ b/be/src/vec/columns/column_array.h @@ -126,6 +126,7 @@ class ColumnArray final : public COWHelper { std::string get_name() const override; const char* get_family_name() const override { return "Array"; } bool is_column_array() const override { return true; } + bool is_variable_length() const override { return true; } MutableColumnPtr clone_resized(size_t size) const override; size_t size() const override; void resize(size_t n) override; @@ -219,7 +220,7 @@ class ColumnArray final : public COWHelper { const uint32_t* indices_end) override; void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK_EQ(size(), self_row + 1); + DCHECK(self_row != 0 && size() == self_row + 1); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_const.h b/be/src/vec/columns/column_const.h index c4ac651c2806bb..1cf36b1420fd7e 100644 --- a/be/src/vec/columns/column_const.h +++ b/be/src/vec/columns/column_const.h @@ -105,6 +105,8 @@ class ColumnConst final : public COWHelper { return convert_to_full_column()->convert_to_full_column_if_const(); } + bool is_variable_length() const override { return data->is_variable_length(); } + ColumnPtr remove_low_cardinality() const; std::string get_name() const override { return "Const(" + data->get_name() + ")"; } diff --git a/be/src/vec/columns/column_map.h b/be/src/vec/columns/column_map.h index 0e6ad3c3d91cf2..81fa0cd9a0a559 100644 --- a/be/src/vec/columns/column_map.h +++ b/be/src/vec/columns/column_map.h @@ -94,6 +94,7 @@ class ColumnMap final : public COWHelper { ColumnPtr convert_to_full_column_if_const() const override; MutableColumnPtr clone_resized(size_t size) const override; + bool is_variable_length() const override { return true; } Field operator[](size_t n) const override; void get(size_t n, Field& res) const override; @@ -136,7 +137,7 @@ class ColumnMap final : public COWHelper { } void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(size() > self_row); + DCHECK(self_row != 0 && size() == self_row + 1); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_key_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_nullable.h b/be/src/vec/columns/column_nullable.h index f097432cf408c7..4e202924fb29dc 100644 --- a/be/src/vec/columns/column_nullable.h +++ b/be/src/vec/columns/column_nullable.h @@ -82,7 +82,7 @@ class ColumnNullable final : public COWHelper { } MutableColumnPtr get_shrinked_column() override; - + bool is_variable_length() const override { return nested_column->is_variable_length(); } const char* get_family_name() const override { return "Nullable"; } std::string get_name() const override { return "Nullable(" + nested_column->get_name() + ")"; } MutableColumnPtr clone_resized(size_t size) const override; diff --git a/be/src/vec/columns/column_string.h b/be/src/vec/columns/column_string.h index f461ff887d2dc5..8e13876614b2d4 100644 --- a/be/src/vec/columns/column_string.h +++ b/be/src/vec/columns/column_string.h @@ -106,7 +106,7 @@ class ColumnString final : public COWHelper { public: void sanity_check() const; - + bool is_variable_length() const override { return true; } const char* get_family_name() const override { return "String"; } size_t size() const override { return offsets.size(); } @@ -548,11 +548,17 @@ class ColumnString final : public COWHelper { } void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK_EQ(size(), self_row + 1); + // we check this column size and self_row because we need to make sure when we call + // replace_column_data() with a batch column data. + // and this column is cleared at the every beginning. next we replace column one by one, so + // the self_row is only equals size() - 1. + DCHECK(self_row != 0 && size() == self_row + 1); const auto& r = assert_cast(rhs); auto data = r.get_data_at(row); if (!self_row) { + // self_row == 0 means we first call replace_column_data() with batch column data. so we + // should clean last batch column data. chars.clear(); offsets[self_row] = data.size; } else { diff --git a/be/src/vec/olap/block_reader.cpp b/be/src/vec/olap/block_reader.cpp index aa8f8861ecb283..e2f37fee0101ac 100644 --- a/be/src/vec/olap/block_reader.cpp +++ b/be/src/vec/olap/block_reader.cpp @@ -193,22 +193,7 @@ void BlockReader::_init_agg_state(const ReaderParams& read_params) { _agg_places.push_back(place); // calculate `_has_variable_length_tag` tag. like string, array, map - _stored_has_variable_length_tag[idx] = - _stored_data_columns[idx]->is_column_string() || - (_stored_data_columns[idx]->is_nullable() && - reinterpret_cast(_stored_data_columns[idx].get()) - ->get_nested_column_ptr() - ->is_column_string()) || - _stored_data_columns[idx]->is_column_array() || - (_stored_data_columns[idx]->is_nullable() && - reinterpret_cast(_stored_data_columns[idx].get()) - ->get_nested_column_ptr() - ->is_column_array()) || - _stored_data_columns[idx]->is_column_map() || - (_stored_data_columns[idx]->is_nullable() && - reinterpret_cast(_stored_data_columns[idx].get()) - ->get_nested_column_ptr() - ->is_column_map()); + _stored_has_variable_length_tag[idx] = _stored_data_columns[idx]->is_variable_length(); } } diff --git a/be/src/vec/olap/vertical_block_reader.cpp b/be/src/vec/olap/vertical_block_reader.cpp index 74ac6d3805f149..4fa518d58ac677 100644 --- a/be/src/vec/olap/vertical_block_reader.cpp +++ b/be/src/vec/olap/vertical_block_reader.cpp @@ -199,22 +199,7 @@ void VerticalBlockReader::_init_agg_state(const ReaderParams& read_params) { _agg_places.push_back(place); // calculate `_has_variable_length_tag` tag. like string, array, map - _stored_has_variable_length_tag[idx] = - _stored_data_columns[idx]->is_column_string() || - (_stored_data_columns[idx]->is_nullable() && - reinterpret_cast(_stored_data_columns[idx].get()) - ->get_nested_column_ptr() - ->is_column_string()) || - _stored_data_columns[idx]->is_column_array() || - (_stored_data_columns[idx]->is_nullable() && - reinterpret_cast(_stored_data_columns[idx].get()) - ->get_nested_column_ptr() - ->is_column_array()) || - _stored_data_columns[idx]->is_column_map() || - (_stored_data_columns[idx]->is_nullable() && - reinterpret_cast(_stored_data_columns[idx].get()) - ->get_nested_column_ptr() - ->is_column_map()); + _stored_has_variable_length_tag[idx] = _stored_data_columns[idx]->is_variable_length(); } } From 02f047fb10c7fea6ec7dbfc4484069c91db0cea5 Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 2 Apr 2024 12:20:29 +0800 Subject: [PATCH 4/5] fixed --- be/src/vec/columns/column_array.h | 2 +- be/src/vec/columns/column_map.h | 2 +- be/src/vec/columns/column_string.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/be/src/vec/columns/column_array.h b/be/src/vec/columns/column_array.h index 08fbb03ce716d8..25a06717ac6f89 100644 --- a/be/src/vec/columns/column_array.h +++ b/be/src/vec/columns/column_array.h @@ -220,7 +220,7 @@ class ColumnArray final : public COWHelper { const uint32_t* indices_end) override; void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(self_row != 0 && size() == self_row + 1); + DCHECK(self_row == 0 || (self_row != 0 && size() == self_row + 1)); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_map.h b/be/src/vec/columns/column_map.h index 81fa0cd9a0a559..0f69d14380a7ad 100644 --- a/be/src/vec/columns/column_map.h +++ b/be/src/vec/columns/column_map.h @@ -137,7 +137,7 @@ class ColumnMap final : public COWHelper { } void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(self_row != 0 && size() == self_row + 1); + DCHECK(self_row == 0 || (self_row != 0 && size() == self_row + 1)); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_key_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_string.h b/be/src/vec/columns/column_string.h index 8e13876614b2d4..aa07892e50be4e 100644 --- a/be/src/vec/columns/column_string.h +++ b/be/src/vec/columns/column_string.h @@ -552,7 +552,7 @@ class ColumnString final : public COWHelper { // replace_column_data() with a batch column data. // and this column is cleared at the every beginning. next we replace column one by one, so // the self_row is only equals size() - 1. - DCHECK(self_row != 0 && size() == self_row + 1); + DCHECK(self_row == 0 || (self_row != 0 && size() == self_row + 1)); const auto& r = assert_cast(rhs); auto data = r.get_data_at(row); From 9f34eb21390bd466e1ad8985b1f3471fdb4b0a98 Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 2 Apr 2024 19:55:21 +0800 Subject: [PATCH 5/5] revert origin dcheck --- be/src/vec/columns/column_array.h | 2 +- be/src/vec/columns/column_map.h | 2 +- be/src/vec/columns/column_string.h | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/be/src/vec/columns/column_array.h b/be/src/vec/columns/column_array.h index 25a06717ac6f89..79580d36a17f30 100644 --- a/be/src/vec/columns/column_array.h +++ b/be/src/vec/columns/column_array.h @@ -220,7 +220,7 @@ class ColumnArray final : public COWHelper { const uint32_t* indices_end) override; void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(self_row == 0 || (self_row != 0 && size() == self_row + 1)); + DCHECK(size() > self_row); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_map.h b/be/src/vec/columns/column_map.h index 0f69d14380a7ad..3b9c4a100e008d 100644 --- a/be/src/vec/columns/column_map.h +++ b/be/src/vec/columns/column_map.h @@ -137,7 +137,7 @@ class ColumnMap final : public COWHelper { } void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { - DCHECK(self_row == 0 || (self_row != 0 && size() == self_row + 1)); + DCHECK(size() > self_row); const auto& r = assert_cast(rhs); const size_t nested_row_size = r.size_at(row); const size_t r_key_nested_start_off = r.offset_at(row); diff --git a/be/src/vec/columns/column_string.h b/be/src/vec/columns/column_string.h index aa07892e50be4e..705af04b1f84a6 100644 --- a/be/src/vec/columns/column_string.h +++ b/be/src/vec/columns/column_string.h @@ -550,9 +550,9 @@ class ColumnString final : public COWHelper { void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { // we check this column size and self_row because we need to make sure when we call // replace_column_data() with a batch column data. - // and this column is cleared at the every beginning. next we replace column one by one, so - // the self_row is only equals size() - 1. - DCHECK(self_row == 0 || (self_row != 0 && size() == self_row + 1)); + // and this column data is cleared at the every beginning. + // next we replace column one by one. + DCHECK(size() > self_row); const auto& r = assert_cast(rhs); auto data = r.get_data_at(row);