This is an automated email from the ASF dual-hosted git repository.
yiguolei 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 50aa052ad95 [refactor](iterator) Clean up VStatisticsIterator and move
VAutoIncrementIterator into test (#69627)
50aa052ad95 is described below
commit 50aa052ad9555fcdd31913584f090608faf35fa7
Author: Chenyang Sun <[email protected]>
AuthorDate: Tue Oct 27 10:49:43 2026 +0800
[refactor](iterator) Clean up VStatisticsIterator and move
VAutoIncrementIterator into test (#69627)
Remove `_column_iterators_map` from VStatisticsIterator and drop its
CHAR padding trim code.
---------
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
be/src/core/string_ref.cpp | 11 --
be/src/core/string_ref.h | 1 -
be/src/storage/iterator/vgeneric_iterators.cpp | 111 +-------------
be/src/storage/iterator/vgeneric_iterators.h | 10 +-
be/test/exec/scan/vgeneric_iterators_test.cpp | 192 ++++++++++++++++++++-----
5 files changed, 164 insertions(+), 161 deletions(-)
diff --git a/be/src/core/string_ref.cpp b/be/src/core/string_ref.cpp
index 6428d6968e0..849de48c9b4 100644
--- a/be/src/core/string_ref.cpp
+++ b/be/src/core/string_ref.cpp
@@ -40,17 +40,6 @@ StringRef StringRef::trim() const {
return StringRef(data + begin, end - begin + 1);
}
-StringRef StringRef::trim_tail_padding_zero() const {
- // Remove trailing padding zero.
- int64_t end = size - 1;
-
- while (end >= 0 && data[end] == '\0') {
- --end;
- }
-
- return StringRef(data, end + 1);
-}
-
StringRef StringRef::trim_whitespace() const {
// Remove leading and trailing whitespace.
int64_t begin = 0;
diff --git a/be/src/core/string_ref.h b/be/src/core/string_ref.h
index cf9cbcf3a37..56764409685 100644
--- a/be/src/core/string_ref.h
+++ b/be/src/core/string_ref.h
@@ -218,7 +218,6 @@ struct StringRef {
// Trims leading and trailing spaces.
StringRef trim() const;
- StringRef trim_tail_padding_zero() const;
StringRef trim_whitespace() const;
StringRef trim_quote() const;
diff --git a/be/src/storage/iterator/vgeneric_iterators.cpp
b/be/src/storage/iterator/vgeneric_iterators.cpp
index b3c7358f418..eacfbbb3df0 100644
--- a/be/src/storage/iterator/vgeneric_iterators.cpp
+++ b/be/src/storage/iterator/vgeneric_iterators.cpp
@@ -44,14 +44,12 @@ Status VStatisticsIterator::init(const StorageReadOptions&
opts) {
if (!_init) {
_push_down_agg_type_opt = opts.push_down_agg_type_opt;
- for (size_t ordinal = 0; ordinal < _schema.num_block_columns();
ordinal++) {
- const auto* column_desc = _schema.column(ordinal);
- auto unique_id = column_desc->unique_id();
- if (_column_iterators_map.count(unique_id) < 1) {
- RETURN_IF_ERROR(_segment->new_column_iterator(
- *column_desc, &_column_iterators_map[unique_id],
&opts));
+ if (_push_down_agg_type_opt != TPushAggOp::COUNT) {
+ _column_iterators.resize(_schema.num_block_columns());
+ for (size_t ordinal = 0; ordinal < _schema.num_block_columns();
++ordinal) {
+
RETURN_IF_ERROR(_segment->new_column_iterator(*_schema.column(ordinal),
+
&_column_iterators[ordinal], &opts));
}
-
_column_iterators.push_back(_column_iterators_map[unique_id].get());
}
_target_rows = _push_down_agg_type_opt == TPushAggOp::MINMAX ? 2 :
_segment->num_rows();
@@ -62,7 +60,7 @@ Status VStatisticsIterator::init(const StorageReadOptions&
opts) {
}
Status VStatisticsIterator::next_batch(Block* block) {
- DCHECK_EQ(block->columns(), _column_iterators.size());
+ DCHECK_EQ(block->columns(), _schema.num_block_columns());
if (_output_rows < _target_rows) {
block->clear_column_data();
auto columns_guard = block->mutate_columns_scoped();
@@ -76,16 +74,8 @@ Status VStatisticsIterator::next_batch(Block* block) {
column->insert_many_defaults(size);
}
} else {
- for (int i = 0; i < columns.size(); ++i) {
+ for (size_t i = 0; i < columns.size(); ++i) {
RETURN_IF_ERROR(_column_iterators[i]->next_batch_of_zone_map(&size,
columns[i]));
- if (_schema.column(i)->type() ==
FieldType::OLAP_FIELD_TYPE_CHAR) {
- auto col = columns[i]->clone_empty();
- for (size_t j = 0; j < columns[i]->size(); ++j) {
- const auto ref =
columns[i]->get_data_at(j).trim_tail_padding_zero();
- col->insert_data(ref.data, ref.size);
- }
- columns[i].swap(col);
- }
}
}
_output_rows += size;
@@ -192,89 +182,6 @@ Status VMergeIteratorContext::copy_rows(BlockView* view,
bool advanced) {
return Status::OK();
}
-// This iterator will generate ordered data. For example for schema
-// (int, int) this iterator will generator data like
-// (0, 1), (1, 2), (2, 3), (3, 4)...
-//
-// Usage:
-// ReadSchema schema;
-// VAutoIncrementIterator iter(schema, 1000);
-// StorageReadOptions opts;
-// RETURN_IF_ERROR(iter.init(opts));
-// Block block;
-// do {
-// st = iter.next_batch(&block);
-// } while (st.ok());
-class VAutoIncrementIterator : public RowwiseIterator {
-public:
- // Will generate num_rows rows in total
- VAutoIncrementIterator(const ReadSchema& schema, size_t num_rows)
- : _schema(schema), _num_rows(num_rows), _rows_returned() {}
- ~VAutoIncrementIterator() override = default;
-
- // NOTE: Currently, this function will ignore StorageReadOptions
- Status init(const StorageReadOptions& opts) override;
-
- Status next_batch(Block* block) override {
- int row_idx = 0;
- while (_rows_returned < _num_rows) {
- for (int j = 0; j < _schema.num_block_columns(); ++j) {
- ColumnWithTypeAndName& vc = block->get_by_position(j);
- IColumn& vi = (IColumn&)(*vc.column);
-
- char data[16] = {};
- size_t data_len = 0;
- const auto* col_schema = _schema.column(j);
- switch (col_schema->type()) {
- case FieldType::OLAP_FIELD_TYPE_SMALLINT:
- *(int16_t*)data = cast_set<int16_t>(_rows_returned + j);
- data_len = sizeof(int16_t);
- break;
- case FieldType::OLAP_FIELD_TYPE_INT:
- *(int32_t*)data = cast_set<int32_t>(_rows_returned + j);
- data_len = sizeof(int32_t);
- break;
- case FieldType::OLAP_FIELD_TYPE_BIGINT:
- *(int64_t*)data = cast_set<int64_t>(_rows_returned + j);
- data_len = sizeof(int64_t);
- break;
- case FieldType::OLAP_FIELD_TYPE_FLOAT:
- *(float*)data = cast_set<float>(_rows_returned + j);
- data_len = sizeof(float);
- break;
- case FieldType::OLAP_FIELD_TYPE_DOUBLE:
- *(double*)data = cast_set<double>(_rows_returned + j);
- data_len = sizeof(double);
- break;
- default:
- break;
- }
-
- vi.insert_data(data, data_len);
- }
-
- ++row_idx;
- ++_rows_returned;
- }
-
- if (row_idx > 0) {
- return Status::OK();
- }
- return Status::EndOfFile("End of VAutoIncrementIterator");
- }
-
- const ReadSchema& schema() const override { return _schema; }
-
-private:
- const ReadSchema& _schema;
- size_t _num_rows;
- size_t _rows_returned;
-};
-
-Status VAutoIncrementIterator::init(const StorageReadOptions& opts) {
- return Status::OK();
-}
-
Status VMergeIteratorContext::init(const StorageReadOptions& opts) {
_block_row_max = opts.block_row_max;
RETURN_IF_ERROR(_load_next_block());
@@ -488,8 +395,4 @@ RowwiseIterator*
new_vstatistics_iterator(std::shared_ptr<Segment> segment,
return new VStatisticsIterator(segment, schema);
}
-RowwiseIteratorUPtr new_auto_increment_iterator(const ReadSchema& schema,
size_t num_rows) {
- return std::make_unique<VAutoIncrementIterator>(schema, num_rows);
-}
-
} // namespace doris
diff --git a/be/src/storage/iterator/vgeneric_iterators.h
b/be/src/storage/iterator/vgeneric_iterators.h
index 876bb620aa9..cc416d9efea 100644
--- a/be/src/storage/iterator/vgeneric_iterators.h
+++ b/be/src/storage/iterator/vgeneric_iterators.h
@@ -21,7 +21,6 @@
#include <stdint.h>
#include <list>
-#include <map>
#include <memory>
#include <queue>
#include <utility>
@@ -64,8 +63,7 @@ private:
size_t _output_rows = 0;
bool _init = false;
TPushAggOp::type _push_down_agg_type_opt;
- std::map<int32_t, std::unique_ptr<ColumnIterator>> _column_iterators_map;
- std::vector<ColumnIterator*> _column_iterators;
+ std::vector<std::unique_ptr<ColumnIterator>> _column_iterators;
static constexpr size_t MAX_ROW_SIZE_IN_COUNT = 65535;
};
@@ -325,12 +323,6 @@ RowwiseIteratorUPtr
new_merge_iterator(std::vector<RowwiseIteratorUPtr>&& inputs
RowwiseIteratorUPtr new_union_iterator(std::vector<RowwiseIteratorUPtr>&&
inputs,
ReadSchemaSPtr read_schema);
-// Create an auto increment iterator which returns num_rows data in format of
schema.
-// This class aims to be used in unit test.
-//
-// Client should delete returned iterator.
-RowwiseIteratorUPtr new_auto_increment_iterator(const ReadSchema& schema,
size_t num_rows);
-
RowwiseIterator* new_vstatistics_iterator(std::shared_ptr<Segment> segment,
const ReadSchema& schema);
diff --git a/be/test/exec/scan/vgeneric_iterators_test.cpp
b/be/test/exec/scan/vgeneric_iterators_test.cpp
index 6e91f5a6941..187e8b486dd 100644
--- a/be/test/exec/scan/vgeneric_iterators_test.cpp
+++ b/be/test/exec/scan/vgeneric_iterators_test.cpp
@@ -22,8 +22,11 @@
#include <cstring>
#include <memory>
+#include <optional>
+#include <string>
#include <vector>
+#include "common/cast_set.h"
#include "core/assert_cast.h"
#include "core/block/column_with_type_and_name.h"
#include "core/column/column.h"
@@ -50,6 +53,73 @@
namespace doris {
using namespace ErrorCode;
+namespace {
+
+// Generates `num_rows` ordered rows in the layout of `schema`: row r holds r
+ j in column j, so
+// (int, int) gives (0, 1), (1, 2), (2, 3), ... It hands every row out in its
first next_batch()
+// and ignores the StorageReadOptions; the tests below rely on both.
+class AutoIncrementUtIterator : public RowwiseIterator {
+public:
+ AutoIncrementUtIterator(const ReadSchema& schema, size_t num_rows)
+ : _schema(schema), _num_rows(num_rows) {}
+ ~AutoIncrementUtIterator() override = default;
+
+ Status init(const StorageReadOptions& opts) override { return
Status::OK(); }
+
+ Status next_batch(Block* block) override {
+ if (_rows_returned == _num_rows) {
+ return Status::EndOfFile("End of AutoIncrementUtIterator");
+ }
+ auto columns_guard = block->mutate_columns_scoped();
+ auto& columns = columns_guard.mutable_columns();
+ for (; _rows_returned < _num_rows; ++_rows_returned) {
+ for (size_t j = 0; j < _schema.num_block_columns(); ++j) {
+ char data[16] = {};
+ size_t data_len = 0;
+ switch (_schema.column(j)->type()) {
+ case FieldType::OLAP_FIELD_TYPE_SMALLINT:
+ *(int16_t*)data = cast_set<int16_t>(_rows_returned + j);
+ data_len = sizeof(int16_t);
+ break;
+ case FieldType::OLAP_FIELD_TYPE_INT:
+ *(int32_t*)data = cast_set<int32_t>(_rows_returned + j);
+ data_len = sizeof(int32_t);
+ break;
+ case FieldType::OLAP_FIELD_TYPE_BIGINT:
+ *(int64_t*)data = cast_set<int64_t>(_rows_returned + j);
+ data_len = sizeof(int64_t);
+ break;
+ case FieldType::OLAP_FIELD_TYPE_FLOAT:
+ *(float*)data = cast_set<float>(_rows_returned + j);
+ data_len = sizeof(float);
+ break;
+ case FieldType::OLAP_FIELD_TYPE_DOUBLE:
+ *(double*)data = cast_set<double>(_rows_returned + j);
+ data_len = sizeof(double);
+ break;
+ default:
+ break;
+ }
+ columns[j]->insert_data(data, data_len);
+ }
+ }
+ return Status::OK();
+ }
+
+ const ReadSchema& schema() const override { return _schema; }
+
+private:
+ const ReadSchema& _schema;
+ size_t _num_rows;
+ size_t _rows_returned = 0;
+};
+
+RowwiseIteratorUPtr new_auto_increment_iterator(const ReadSchema& schema,
size_t num_rows) {
+ return std::make_unique<AutoIncrementUtIterator>(schema, num_rows);
+}
+
+} // namespace
+
class VGenericIteratorsTest : public testing::Test {
public:
VGenericIteratorsTest() {}
@@ -119,17 +189,15 @@ TEST(VGenericIteratorsTest, AutoIncrement) {
}
}
-TEST(VGenericIteratorsTest, StatisticsIteratorPreservesNullForNullableChar) {
- constexpr auto test_dir = "./ut_dir/vgeneric_iterators_test";
- constexpr auto segment_path =
"./ut_dir/vgeneric_iterators_test/nullable_char_segment.dat";
- constexpr auto row_count = 3;
-
+// Writes one segment of an INT key and a nullable CHAR(8) column holding
`values`, std::nullopt
+// for NULL, and opens it. The writer pads every CHAR value to the declared 8
bytes, which is what
+// the segment's zone map keeps as its bounds.
+static std::shared_ptr<segment_v2::Segment> write_char_segment(
+ const std::string& segment_path, const
std::vector<std::optional<std::string>>& values,
+ TabletSchemaSPtr* tablet_schema) {
auto fs = io::global_local_filesystem();
- ASSERT_TRUE(fs->delete_directory(test_dir).ok());
- ASSERT_TRUE(fs->create_directory(test_dir).ok());
-
- auto tablet_schema = std::make_shared<TabletSchema>();
- tablet_schema->append_column(*create_int_key(0, false));
+ *tablet_schema = std::make_shared<TabletSchema>();
+ (*tablet_schema)->append_column(*create_int_key(0, false));
auto nullable_char = std::make_shared<TabletColumn>();
nullable_char->set_unique_id(1);
nullable_char->set_name("1");
@@ -138,55 +206,107 @@ TEST(VGenericIteratorsTest,
StatisticsIteratorPreservesNullForNullableChar) {
nullable_char->set_length(8);
nullable_char->set_index_length(8);
nullable_char->set_aggregation_method(FieldAggregationMethod::OLAP_FIELD_AGGREGATION_NONE);
- tablet_schema->append_column(*nullable_char);
- tablet_schema->set_storage_page_size(4096);
+ (*tablet_schema)->append_column(*nullable_char);
+ (*tablet_schema)->set_storage_page_size(4096);
io::FileWriterPtr file_writer;
- ASSERT_TRUE(fs->create_file(segment_path, &file_writer).ok());
+ EXPECT_TRUE(fs->create_file(segment_path, &file_writer).ok());
VerticalSegmentWriterOptions writer_options;
writer_options.num_rows_per_block = 1024;
- TestVerticalSegmentWriter writer(file_writer.get(), 0, tablet_schema,
nullptr, nullptr,
+ TestVerticalSegmentWriter writer(file_writer.get(), 0, *tablet_schema,
nullptr, nullptr,
writer_options, nullptr);
- ASSERT_TRUE(writer.init().ok());
+ EXPECT_TRUE(writer.init().ok());
RowCursor row;
- std::vector<Field> fields(tablet_schema->num_columns(),
Field(PrimitiveType::TYPE_NULL));
- ASSERT_TRUE(row.init_scan_key(tablet_schema, std::move(fields)).ok());
- for (int i = 0; i < row_count; ++i) {
- row.mutable_field(0) = Field::create_field<TYPE_INT>(i);
- ASSERT_TRUE(writer.append_row(row).ok());
+ std::vector<Field> fields((*tablet_schema)->num_columns(),
Field(PrimitiveType::TYPE_NULL));
+ EXPECT_TRUE(row.init_scan_key(*tablet_schema, std::move(fields)).ok());
+ for (size_t i = 0; i < values.size(); ++i) {
+ row.mutable_field(0) =
Field::create_field<TYPE_INT>(static_cast<int32_t>(i));
+ row.mutable_field(1) = values[i].has_value()
+ ?
Field::create_field<TYPE_STRING>(String(*values[i]))
+ : Field(PrimitiveType::TYPE_NULL);
+ EXPECT_TRUE(writer.append_row(row).ok());
}
uint64_t file_size = 0;
uint64_t index_size = 0;
- ASSERT_TRUE(writer.finalize_columns(&index_size).ok());
- ASSERT_TRUE(writer.finalize_footer(&file_size).ok());
- ASSERT_TRUE(file_writer->close().ok());
+ EXPECT_TRUE(writer.finalize_columns(&index_size).ok());
+ EXPECT_TRUE(writer.finalize_footer(&file_size).ok());
+ EXPECT_TRUE(file_writer->close().ok());
std::shared_ptr<segment_v2::Segment> segment;
- ASSERT_TRUE(segment_v2::Segment::open(fs, segment_path, 100, 0, RowsetId
{.version = 1},
- tablet_schema, io::FileReaderOptions
{}, &segment)
+ EXPECT_TRUE(segment_v2::Segment::open(fs, segment_path, 100, 0, RowsetId
{.version = 1},
+ *tablet_schema,
io::FileReaderOptions {}, &segment)
.ok());
+ return segment;
+}
- std::vector<ColumnId> column_ids {0, 1};
- ReadSchema schema(project_columns_by_ordinal(tablet_schema->columns(),
column_ids));
- VStatisticsIterator iterator(segment, schema);
- OlapReaderStatistics stats;
- StorageReadOptions read_options(stats);
- read_options.push_down_agg_type_opt = TPushAggOp::MINMAX;
- ASSERT_TRUE(iterator.init(read_options).ok());
+class StatisticsIteratorCharTest : public testing::Test {
+protected:
+ static constexpr auto kTestDir = "./ut_dir/vgeneric_iterators_test";
+
+ void SetUp() override {
+ _fs = io::global_local_filesystem();
+ ASSERT_TRUE(_fs->delete_directory(kTestDir).ok());
+ ASSERT_TRUE(_fs->create_directory(kTestDir).ok());
+ }
+ void TearDown() override {
EXPECT_TRUE(_fs->delete_directory(kTestDir).ok()); }
+
+ // Answers the pushed-down `agg` over a segment holding `values` in the
CHAR column: the
+ // first Block it hands out goes to `block`, and the read must be over
after it.
+ void read_statistics(const std::vector<std::optional<std::string>>& values,
+ TPushAggOp::type agg, Block* block) {
+ TabletSchemaSPtr tablet_schema;
+ auto segment = write_char_segment(std::string(kTestDir) +
"/char_segment.dat", values,
+ &tablet_schema);
+ ASSERT_NE(segment, nullptr);
+ std::vector<ColumnId> column_ids {0, 1};
+ ReadSchema schema(project_columns_by_ordinal(tablet_schema->columns(),
column_ids));
+ std::unique_ptr<RowwiseIterator> iterator =
+ std::make_unique<VStatisticsIterator>(segment, schema);
+ OlapReaderStatistics stats;
+ StorageReadOptions read_options(stats);
+ read_options.push_down_agg_type_opt = agg;
+ ASSERT_TRUE(iterator->init(read_options).ok());
+
+ create_block(schema, *block);
+ ASSERT_TRUE(iterator->next_batch(block).ok());
+ Block after_last;
+ create_block(schema, after_last);
+
ASSERT_TRUE(iterator->next_batch(&after_last).is<ErrorCode::END_OF_FILE>());
+ }
+
+ std::shared_ptr<io::FileSystem> _fs;
+};
+TEST_F(StatisticsIteratorCharTest, PreservesNullForNullableChar) {
Block block;
- create_block(schema, block);
- ASSERT_TRUE(iterator.next_batch(&block).ok());
+ read_statistics({std::nullopt, std::nullopt, std::nullopt},
TPushAggOp::MINMAX, &block);
ASSERT_EQ(2, block.rows());
const auto& nullable_column =
assert_cast<const
ColumnNullable&>(*block.get_by_position(1).column);
EXPECT_TRUE(nullable_column.is_null_at(0));
EXPECT_TRUE(nullable_column.is_null_at(1));
- ASSERT_TRUE(iterator.next_batch(&block).is<ErrorCode::END_OF_FILE>());
+}
- ASSERT_TRUE(fs->delete_directory(test_dir).ok());
+// CHAR(8) stores "a" as "a" plus seven '\0'. MIN/MAX answered from the zone
map must come out as
+// the rows of a normal read do: without the padding.
+TEST_F(StatisticsIteratorCharTest, AnswersCharBoundsWithoutPadding) {
+ Block block;
+ read_statistics({"bb", "a", "ccc"}, TPushAggOp::MINMAX, &block);
+ ASSERT_EQ(2, block.rows());
+
+ // next_batch_of_zone_map hands out the max first, then the min.
+ const auto& char_column = *block.get_by_position(1).column;
+ EXPECT_EQ("ccc", char_column.get_data_at(0).to_string());
+ EXPECT_EQ("a", char_column.get_data_at(1).to_string());
+}
+
+// COUNT reports the segment's row count and reads no zone map, so every row
is a default.
+TEST_F(StatisticsIteratorCharTest, CountFillsTheRowCount) {
+ Block block;
+ read_statistics({"bb", std::nullopt, "ccc"}, TPushAggOp::COUNT, &block);
+ EXPECT_EQ(3, block.rows());
}
// A string zone map bound is cut to 512 bytes, and a cut bound is not a value
the column holds:
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
