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 0cf08938ab2 [fix](be) Fix missing decimal precision and scale for
legacy decimal data (#65419)
0cf08938ab2 is described below
commit 0cf08938ab2d3cede6a9136d1bea45d2b1fd9638
Author: TengJianPing <[email protected]>
AuthorDate: Mon Jul 13 10:27:00 2026 +0800
[fix](be) Fix missing decimal precision and scale for legacy decimal data
(#65419)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #26572, #63569
Problem Summary:
Segments written before decimal ColumnMetaPB precision and scale were
persisted can miss precision and scale for decimal types in metadata .
Regular columns, complex decimal children may then build decimal
DataType instances without the logical precision/scale from
TabletSchema, and result in error "Wrong precision 0, min: 1, max: 76".
This change centralizes decimal metadata recovery from TabletSchema,
applies it before ColumnReader/DataType creation.
### Release note
Fix reading legacy decimal whose segment metadata is missing decimal
precision and scale.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
be/src/core/data_type/data_type_decimal.h | 13 +-
be/src/core/data_type/data_type_factory.cpp | 19 +-
.../index/inverted/inverted_index_iterator.cpp | 2 +-
be/src/storage/olap_common.h | 8 +
be/src/storage/segment/column_reader.cpp | 2 +-
be/src/storage/segment/segment.cpp | 47 +++++
be/src/storage/utils.h | 26 ---
be/test/core/data_type/data_type_decimal_test.cpp | 170 ---------------
.../storage/segment/column_reader_cache_test.cpp | 231 ++++++++++++++++++++-
be/test/storage/segment/mock/mock_segment.h | 21 +-
10 files changed, 315 insertions(+), 224 deletions(-)
diff --git a/be/src/core/data_type/data_type_decimal.h
b/be/src/core/data_type/data_type_decimal.h
index 0aac381d16f..69fae16be82 100644
--- a/be/src/core/data_type/data_type_decimal.h
+++ b/be/src/core/data_type/data_type_decimal.h
@@ -207,10 +207,15 @@ public:
static constexpr size_t max_precision() { return
max_decimal_precision<T>(); }
- DataTypeDecimal(UInt32 arg_precision = max_decimal_precision<T>(),
- UInt32 arg_scale = default_decimal_scale<T>(),
- UInt32 arg_original_precision = UINT32_MAX,
- UInt32 arg_original_scale = UINT32_MAX)
+ DataTypeDecimal(
+ UInt32 arg_precision = max_decimal_precision<T>(),
+ UInt32 arg_scale = default_decimal_scale<T>(),
+ // For decimalv2 only, record the original(schema) precision and
scale.
+ // UINT32_MAX means original precision and scale are unknown.
+ // Decimalv2 will be converted to Decimal(27, 9) in memory when
doing any calculations,
+ // but when casting decimalv2 to string, it's better to keep the
presion and
+ // scale of it's original value in schema.
+ UInt32 arg_original_precision = UINT32_MAX, UInt32
arg_original_scale = UINT32_MAX)
: DecimalScaleInfo<T>(arg_precision, arg_scale,
arg_original_precision,
arg_original_scale) {}
diff --git a/be/src/core/data_type/data_type_factory.cpp
b/be/src/core/data_type/data_type_factory.cpp
index ab6551d23ad..256493cba04 100644
--- a/be/src/core/data_type/data_type_factory.cpp
+++ b/be/src/core/data_type/data_type_factory.cpp
@@ -379,23 +379,8 @@ DataTypePtr DataTypeFactory::create_data_type(const
segment_v2::ColumnMetaPB& pc
nested = std::make_shared<DataTypeStruct>(dataTypes, names);
} else {
// TODO add precision and frac
- auto meta_precision = pcolumn.precision();
- auto meta_scale = pcolumn.frac();
- if (pcolumn.type() ==
static_cast<int>(FieldType::OLAP_FIELD_TYPE_DECIMAL)) {
- // Segments written by Doris < 2.1.0 (before #26572) do not persist
- // precision/frac in ColumnMetaPB, so they default to 0 when read
back.
- // Pass UINT32_MAX to DataTypeDecimalV2 to signal that the original
- // precision/scale are unknown; otherwise check_type_precision(0)
throws
- // "meet invalid precision: real_precision=0".
- UInt32 orig_precision =
- meta_precision > 0 ? static_cast<UInt32>(meta_precision) :
UINT32_MAX;
- UInt32 orig_scale = meta_precision > 0 ?
static_cast<UInt32>(meta_scale) : UINT32_MAX;
- nested =
_create_primitive_data_type(static_cast<FieldType>(pcolumn.type()),
- orig_precision, orig_scale,
-1);
- } else {
- nested =
_create_primitive_data_type(static_cast<FieldType>(pcolumn.type()),
- meta_precision, meta_scale,
-1);
- }
+ nested =
_create_primitive_data_type(static_cast<FieldType>(pcolumn.type()),
+ pcolumn.precision(),
pcolumn.frac(), -1);
}
if (pcolumn.is_nullable() && nested) {
diff --git a/be/src/storage/index/inverted/inverted_index_iterator.cpp
b/be/src/storage/index/inverted/inverted_index_iterator.cpp
index 936b82d5d56..6e93d70d025 100644
--- a/be/src/storage/index/inverted/inverted_index_iterator.cpp
+++ b/be/src/storage/index/inverted/inverted_index_iterator.cpp
@@ -273,7 +273,7 @@ Result<InvertedIndexReaderPtr>
InvertedIndexIterator::select_best_reader(
return select_for_text(match, query_type, normalized_key);
}
- if (is_numeric_type(field_type)) {
+ if (field_is_numeric_type(field_type)) {
return select_for_numeric(match, query_type);
}
diff --git a/be/src/storage/olap_common.h b/be/src/storage/olap_common.h
index 12318a02905..b34f6ce15ea 100644
--- a/be/src/storage/olap_common.h
+++ b/be/src/storage/olap_common.h
@@ -199,6 +199,14 @@ constexpr bool field_is_slice_type(const FieldType&
field_type) {
field_type == FieldType::OLAP_FIELD_TYPE_STRING;
}
+constexpr bool field_is_decimal_type(const FieldType& field_type) {
+ return field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL ||
+ field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL32 ||
+ field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL64 ||
+ field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL128I ||
+ field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL256;
+}
+
constexpr bool field_is_numeric_type(const FieldType& field_type) {
return field_type == FieldType::OLAP_FIELD_TYPE_INT ||
field_type == FieldType::OLAP_FIELD_TYPE_UNSIGNED_INT ||
diff --git a/be/src/storage/segment/column_reader.cpp
b/be/src/storage/segment/column_reader.cpp
index f15e7462e6c..2021793be20 100644
--- a/be/src/storage/segment/column_reader.cpp
+++ b/be/src/storage/segment/column_reader.cpp
@@ -740,7 +740,7 @@ Status ColumnReader::_load_index(const
std::shared_ptr<IndexFileReader>& index_f
"create StringTypeInvertedIndexReader error: {}",
e.what());
}
}
- } else if (is_numeric_type(type)) {
+ } else if (field_is_numeric_type(type)) {
try {
index_reader = BkdIndexReader::create_shared(index_meta,
index_file_reader);
} catch (const CLuceneError& e) {
diff --git a/be/src/storage/segment/segment.cpp
b/be/src/storage/segment/segment.cpp
index 251452babe5..054f21cec6f 100644
--- a/be/src/storage/segment/segment.cpp
+++ b/be/src/storage/segment/segment.cpp
@@ -144,6 +144,49 @@ Status build_segment_zonemap_context(Segment* segment,
const Schema& schema,
return Status::OK();
}
+void fill_missing_decimal_precision(const TabletColumn& column, ColumnMetaPB*
meta) {
+ auto meta_type = static_cast<FieldType>(meta->type());
+ if (meta_type != column.type()) {
+ return;
+ }
+
+ if (field_is_decimal_type(meta_type)) {
+ if ((!meta->has_precision() || meta->precision() <= 0) &&
column.precision() > 0) {
+ meta->set_precision(column.precision());
+ }
+ if ((!meta->has_frac() || meta->frac() < 0) && column.frac() >= 0) {
+ meta->set_frac(column.frac());
+ }
+ }
+
+ // Complex column meta may also include storage helper children, such as
+ // array offsets. Only schema children have matching TabletColumn subtypes.
+ int child_count =
+ std::min(meta->children_columns_size(),
static_cast<int>(column.get_subtype_count()));
+ for (int i = 0; i < child_count; ++i) {
+ fill_missing_decimal_precision(column.get_sub_column(i),
meta->mutable_children_columns(i));
+ }
+}
+
+void fill_missing_decimal_precision_from_schema(const TabletSchemaSPtr&
tablet_schema,
+ ColumnMetaPB* meta) {
+ if (!meta->has_unique_id()) {
+ return;
+ }
+ int32_t col_idx =
tablet_schema->field_index(static_cast<int32_t>(meta->unique_id()));
+ if (col_idx < 0) {
+ return;
+ }
+ fill_missing_decimal_precision(tablet_schema->column(col_idx), meta);
+}
+
+void fill_footer_missing_decimal_precision(const TabletSchemaSPtr&
tablet_schema,
+ SegmentFooterPB* footer) {
+ for (int i = 0; i < footer->columns_size(); ++i) {
+ fill_missing_decimal_precision_from_schema(tablet_schema,
footer->mutable_columns(i));
+ }
+}
+
} // namespace
Status Segment::open(io::FileSystemSPtr fs, const std::string& path, int64_t
tablet_id,
@@ -604,6 +647,10 @@ Status
Segment::_parse_footer(std::shared_ptr<SegmentFooterPB>& footer,
_file_reader->path().native(), file_size,
file_cache_key_str(_file_reader->path().native()));
}
+ // Segments written before #26572 do not persist decimal precision/frac in
+ // ColumnMetaPB, so recover the logical p/s from TabletSchema before
+ // ColumnReader builds DataTypeDecimal.
+ fill_footer_missing_decimal_precision(_tablet_schema, footer.get());
VLOG_DEBUG << fmt::format("Loading segment footer from {} finished",
_file_reader->path().native());
diff --git a/be/src/storage/utils.h b/be/src/storage/utils.h
index 9772be32589..69c7ddbff00 100644
--- a/be/src/storage/utils.h
+++ b/be/src/storage/utils.h
@@ -185,32 +185,6 @@ constexpr bool is_string_type(const FieldType& field_type)
{
field_type == FieldType::OLAP_FIELD_TYPE_STRING;
}
-constexpr bool is_numeric_type(const FieldType& field_type) {
- return field_type == FieldType::OLAP_FIELD_TYPE_INT ||
- field_type == FieldType::OLAP_FIELD_TYPE_UNSIGNED_INT ||
- field_type == FieldType::OLAP_FIELD_TYPE_BIGINT ||
- field_type == FieldType::OLAP_FIELD_TYPE_SMALLINT ||
- field_type == FieldType::OLAP_FIELD_TYPE_UNSIGNED_TINYINT ||
- field_type == FieldType::OLAP_FIELD_TYPE_UNSIGNED_SMALLINT ||
- field_type == FieldType::OLAP_FIELD_TYPE_TINYINT ||
- field_type == FieldType::OLAP_FIELD_TYPE_DOUBLE ||
- field_type == FieldType::OLAP_FIELD_TYPE_FLOAT ||
- field_type == FieldType::OLAP_FIELD_TYPE_DATE ||
- field_type == FieldType::OLAP_FIELD_TYPE_DATEV2 ||
- field_type == FieldType::OLAP_FIELD_TYPE_DATETIME ||
- field_type == FieldType::OLAP_FIELD_TYPE_DATETIMEV2 ||
- field_type == FieldType::OLAP_FIELD_TYPE_TIMESTAMPTZ ||
- field_type == FieldType::OLAP_FIELD_TYPE_LARGEINT ||
- field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL ||
- field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL32 ||
- field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL64 ||
- field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL128I ||
- field_type == FieldType::OLAP_FIELD_TYPE_DECIMAL256 ||
- field_type == FieldType::OLAP_FIELD_TYPE_BOOL ||
- field_type == FieldType::OLAP_FIELD_TYPE_IPV4 ||
- field_type == FieldType::OLAP_FIELD_TYPE_IPV6;
-}
-
// Util used to get string name of thrift enum item
#define EnumToString(enum_type, index, out) \
do { \
diff --git a/be/test/core/data_type/data_type_decimal_test.cpp
b/be/test/core/data_type/data_type_decimal_test.cpp
index 952f8acb366..64108bb31e7 100644
--- a/be/test/core/data_type/data_type_decimal_test.cpp
+++ b/be/test/core/data_type/data_type_decimal_test.cpp
@@ -454,176 +454,6 @@ TEST_F(DataTypeDecimalTest, ser_deser) {
test_func(dt_decimal256_2, *column_decimal256_2,
BeExecVersionManager::max_be_exec_version);
test_func(dt_decimal256_3, *column_decimal256_3,
BeExecVersionManager::max_be_exec_version);
}
-// Regression for legacy DecimalV2 segments written by Doris < 2.1.0 (before
-// #26572): ColumnMetaPB.precision/frac are absent and default to 0 when read
-// back. DataTypeFactory must not pass 0 as the original precision/scale to
-// DataTypeDecimalV2, otherwise check_type_precision throws
-// "meet invalid precision: real_precision=0".
-TEST_F(DataTypeDecimalTest, create_decimalv2_from_legacy_tablet_column) {
- // Case 1: legacy segment — precision/frac missing (default 0)
- {
- TabletColumn col;
- col.set_type(FieldType::OLAP_FIELD_TYPE_DECIMAL);
- // intentionally do not set precision/frac to mimic old segments
- DataTypePtr dt;
- EXPECT_NO_THROW(dt = DataTypeFactory::instance().create_data_type(col,
false));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_DECIMALV2);
- const auto* decv2 = assert_cast<const DataTypeDecimalV2*>(dt.get());
- EXPECT_EQ(decv2->get_precision(), BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- // When original precision/scale are unknown, get_original_* should
fall
- // back to the in-memory (27, 9) representation.
- EXPECT_EQ(decv2->get_original_precision(),
BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_original_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- }
- // Case 2: new segment — precision/frac are persisted (e.g. decimal(26,6))
- {
- TabletColumn col;
- col.set_type(FieldType::OLAP_FIELD_TYPE_DECIMAL);
- col.set_precision(26);
- col.set_frac(6);
- DataTypePtr dt;
- EXPECT_NO_THROW(dt = DataTypeFactory::instance().create_data_type(col,
false));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_DECIMALV2);
- const auto* decv2 = assert_cast<const DataTypeDecimalV2*>(dt.get());
- EXPECT_EQ(decv2->get_precision(), BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- EXPECT_EQ(decv2->get_original_precision(), 26U);
- EXPECT_EQ(decv2->get_original_scale(), 6U);
- }
- // Case 3: same regression via the segment-read path that consumes
- // ColumnMetaPB directly (this is the exact code path that broke after
- // PR #35222 when reading old DecimalV2 segments).
- {
- segment_v2::ColumnMetaPB meta;
- meta.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_DECIMAL));
- // precision/frac intentionally unset -> default 0
- DataTypePtr dt;
- EXPECT_NO_THROW(dt =
DataTypeFactory::instance().create_data_type(meta));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_DECIMALV2);
- const auto* decv2 = assert_cast<const DataTypeDecimalV2*>(dt.get());
- EXPECT_EQ(decv2->get_precision(), BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- EXPECT_EQ(decv2->get_original_precision(),
BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_original_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- }
-}
-
-// Regression for complex types wrapping legacy DecimalV2 children whose
-// segment ColumnMetaPB.precision/frac default to 0. The fix in
-// DataTypeFactory::create_data_type(segment_v2::ColumnMetaPB&) must
-// propagate through Array / Map / Struct / AggState recursively.
-TEST_F(DataTypeDecimalTest, create_complex_types_with_legacy_decimalv2) {
- auto make_legacy_decv2_meta = []() {
- segment_v2::ColumnMetaPB child;
- child.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_DECIMAL));
- // precision/frac intentionally unset -> default 0
- return child;
- };
- auto unwrap_nullable = [](const DataTypePtr& dt) -> DataTypePtr {
- if (dt && dt->is_nullable()) {
- return assert_cast<const
DataTypeNullable*>(dt.get())->get_nested_type();
- }
- return dt;
- };
-
- // Array<DecimalV2>
- {
- segment_v2::ColumnMetaPB meta;
- meta.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_ARRAY));
- *meta.add_children_columns() = make_legacy_decv2_meta();
- DataTypePtr dt;
- EXPECT_NO_THROW(dt =
DataTypeFactory::instance().create_data_type(meta));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_ARRAY);
- const auto* arr = assert_cast<const DataTypeArray*>(dt.get());
- auto elem = unwrap_nullable(arr->get_nested_type());
- EXPECT_EQ(elem->get_primitive_type(), TYPE_DECIMALV2);
- const auto* decv2 = assert_cast<const DataTypeDecimalV2*>(elem.get());
- EXPECT_EQ(decv2->get_original_precision(),
BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_original_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- }
-
- // Map<Int, DecimalV2>
- {
- segment_v2::ColumnMetaPB meta;
- meta.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_MAP));
- auto* k = meta.add_children_columns();
- k->set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_INT));
- *meta.add_children_columns() = make_legacy_decv2_meta();
- DataTypePtr dt;
- EXPECT_NO_THROW(dt =
DataTypeFactory::instance().create_data_type(meta));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_MAP);
- const auto* m = assert_cast<const DataTypeMap*>(dt.get());
- EXPECT_EQ(unwrap_nullable(m->get_key_type())->get_primitive_type(),
TYPE_INT);
- auto v = unwrap_nullable(m->get_value_type());
- EXPECT_EQ(v->get_primitive_type(), TYPE_DECIMALV2);
- const auto* decv2 = assert_cast<const DataTypeDecimalV2*>(v.get());
- EXPECT_EQ(decv2->get_original_precision(),
BeConsts::MAX_DECIMALV2_PRECISION);
- }
-
- // Struct<DecimalV2, Int>
- {
- segment_v2::ColumnMetaPB meta;
- meta.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_STRUCT));
- *meta.add_children_columns() = make_legacy_decv2_meta();
- auto* second = meta.add_children_columns();
- second->set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_INT));
- DataTypePtr dt;
- EXPECT_NO_THROW(dt =
DataTypeFactory::instance().create_data_type(meta));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_STRUCT);
- const auto* s = assert_cast<const DataTypeStruct*>(dt.get());
- ASSERT_EQ(s->get_elements().size(), 2u);
- EXPECT_EQ(unwrap_nullable(s->get_element(0))->get_primitive_type(),
TYPE_DECIMALV2);
- EXPECT_EQ(unwrap_nullable(s->get_element(1))->get_primitive_type(),
TYPE_INT);
- }
-
- // Nested: Array<Map<Int, DecimalV2>>
- {
- segment_v2::ColumnMetaPB meta;
- meta.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_ARRAY));
- auto* map_child = meta.add_children_columns();
- map_child->set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_MAP));
- auto* k = map_child->add_children_columns();
- k->set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_INT));
- *map_child->add_children_columns() = make_legacy_decv2_meta();
- DataTypePtr dt;
- EXPECT_NO_THROW(dt =
DataTypeFactory::instance().create_data_type(meta));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_ARRAY);
- auto inner =
- unwrap_nullable(assert_cast<const
DataTypeArray*>(dt.get())->get_nested_type());
- EXPECT_EQ(inner->get_primitive_type(), TYPE_MAP);
- auto v = unwrap_nullable(assert_cast<const
DataTypeMap*>(inner.get())->get_value_type());
- EXPECT_EQ(v->get_primitive_type(), TYPE_DECIMALV2);
- }
-
- // AggState (count) with legacy DecimalV2 sub-type
- {
- segment_v2::ColumnMetaPB meta;
- meta.set_type(static_cast<int>(FieldType::OLAP_FIELD_TYPE_AGG_STATE));
- meta.set_function_name("count");
- meta.set_result_is_nullable(false);
- meta.set_be_exec_version(BeExecVersionManager::get_newest_version());
- *meta.add_children_columns() = make_legacy_decv2_meta();
- DataTypePtr dt;
- EXPECT_NO_THROW(dt =
DataTypeFactory::instance().create_data_type(meta));
- ASSERT_NE(dt, nullptr);
- EXPECT_EQ(dt->get_primitive_type(), TYPE_AGG_STATE);
- const auto* agg = assert_cast<const DataTypeAggState*>(dt.get());
- ASSERT_EQ(agg->get_sub_types().size(), 1u);
- auto sub = unwrap_nullable(agg->get_sub_types()[0]);
- EXPECT_EQ(sub->get_primitive_type(), TYPE_DECIMALV2);
- const auto* decv2 = assert_cast<const DataTypeDecimalV2*>(sub.get());
- EXPECT_EQ(decv2->get_original_precision(),
BeConsts::MAX_DECIMALV2_PRECISION);
- EXPECT_EQ(decv2->get_original_scale(), BeConsts::MAX_DECIMALV2_SCALE);
- }
-}
TEST_F(DataTypeDecimalTest, to_pb_column_meta) {
auto test_func = [](auto dt, PGenericType_TypeId expected_type) {
diff --git a/be/test/storage/segment/column_reader_cache_test.cpp
b/be/test/storage/segment/column_reader_cache_test.cpp
index da8aa13d3bf..e080e5f6687 100644
--- a/be/test/storage/segment/column_reader_cache_test.cpp
+++ b/be/test/storage/segment/column_reader_cache_test.cpp
@@ -16,24 +16,32 @@
// under the License.
#include "storage/segment/column_reader_cache.h"
+#include <crc32c/crc32c.h>
#include <gen_cpp/segment_v2.pb.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <chrono>
#include <memory>
+#include <string>
#include <thread>
#include <vector>
#include "common/config.h"
+#include "core/assert_cast.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_nullable.h"
#include "io/fs/file_reader.h"
+#include "io/fs/local_file_system.h"
#include "storage/segment/column_meta_accessor.h"
#include "storage/segment/column_reader.h"
#include "storage/segment/encoding_info.h"
#include "storage/segment/mock/mock_segment.h"
#include "storage/segment/segment.h"
+#include "storage/segment/segment_writer.h"
#include "storage/segment/variant/variant_column_reader.h"
#include "storage/tablet/tablet_schema.h"
+#include "util/coding.h"
#include "util/json/path_in_data.h"
namespace doris::segment_v2 {
@@ -42,6 +50,25 @@ using ::testing::_;
using ::testing::Return;
using ::testing::StrictMock;
+namespace {
+
+constexpr std::string_view kTestDir = "./ut_dir/column_reader_cache_test";
+
+Status append_footer_trailer(io::FileWriter* file_writer, SegmentFooterPB*
footer) {
+ std::string footer_buf;
+ if (!footer->SerializeToString(&footer_buf)) {
+ return Status::InternalError("failed to serialize SegmentFooterPB for
test");
+ }
+ faststring fixed_buf;
+ put_fixed32_le(&fixed_buf, static_cast<uint32_t>(footer_buf.size()));
+ put_fixed32_le(&fixed_buf, crc32c::Crc32c(footer_buf.data(),
footer_buf.size()));
+ fixed_buf.append(k_segment_magic, k_segment_magic_length);
+ std::vector<Slice> slices {Slice(footer_buf), Slice(fixed_buf)};
+ return file_writer->appendv(slices.data(), slices.size());
+}
+
+} // namespace
+
// Mock classes for testing
class MockColumnReader : public ColumnReader {
public:
@@ -69,6 +96,9 @@ protected:
void SetUp() override {
// Set up test configuration
config::max_segment_partial_column_cache_size = 3;
+ auto fs = io::global_local_filesystem();
+ static_cast<void>(fs->delete_directory(kTestDir));
+ CHECK(fs->create_directory(kTestDir).ok());
// Create mock segment
_mock_segment = std::make_unique<StrictMock<MockSegment>>();
@@ -94,6 +124,7 @@ protected:
void TearDown() override {
_cache.reset();
_mock_segment.reset();
+
static_cast<void>(io::global_local_filesystem()->delete_directory(kTestDir));
}
void setup_basic_mocks() {
@@ -132,10 +163,36 @@ protected:
CHECK(_accessor.init(*footer, nullptr).ok());
}
+ void setup_parsed_segment_footer(const std::vector<ColumnMetaPB>& columns)
{
+ SegmentFooterPB footer;
+ for (const auto& col : columns) {
+ *footer.add_columns() = col;
+ }
+
+ auto fs = io::global_local_filesystem();
+ std::string file_path = std::string(kTestDir) + "/parsed_footer_" +
+ std::to_string(_parsed_footer_file_id++) +
".dat";
+ io::FileWriterPtr file_writer;
+ CHECK(fs->create_file(file_path, &file_writer).ok());
+ CHECK(append_footer_trailer(file_writer.get(), &footer).ok());
+ CHECK(file_writer->close().ok());
+
+ io::FileReaderSPtr file_reader;
+ io::FileReaderOptions opts;
+ CHECK(fs->open_file(file_path, &file_reader, &opts).ok());
+ _mock_segment->set_file_reader_for_test(file_reader);
+
+ std::shared_ptr<SegmentFooterPB> parsed_footer;
+ CHECK(_mock_segment->parse_footer_for_test(parsed_footer,
&_stats).ok());
+ _mock_segment->set_footer(parsed_footer);
+ CHECK(_accessor.init(*parsed_footer, nullptr).ok());
+ }
+
std::unique_ptr<StrictMock<MockSegment>> _mock_segment;
ColumnMetaAccessor _accessor;
std::unique_ptr<ColumnReaderCache> _cache;
OlapReaderStatistics _stats;
+ int _parsed_footer_file_id = 0;
};
// Test basic cache functionality
@@ -490,4 +547,176 @@ TEST_F(ColumnReaderCacheTest, EmptyPath) {
EXPECT_TRUE(status.is<ErrorCode::NOT_FOUND>());
}
-} // namespace doris::segment_v2
\ No newline at end of file
+TEST_F(ColumnReaderCacheTest, FillMissingDecimalV2PrecisionFromTabletSchema) {
+ constexpr int32_t col_uid = 10;
+ setup_column_uid_mapping(col_uid, 0);
+ auto& tablet_column =
_mock_segment->tablet_schema()->mutable_column_by_uid(col_uid);
+ tablet_column.set_type(FieldType::OLAP_FIELD_TYPE_DECIMAL);
+ tablet_column.set_precision(20);
+ tablet_column.set_frac(6);
+
+ ColumnMetaPB col_meta;
+
col_meta.set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_DECIMAL));
+ col_meta.set_unique_id(col_uid);
+
col_meta.set_encoding(get_v2_default_encoding(static_cast<FieldType>(col_meta.type())));
+ col_meta.mutable_indexes()->Add()->set_type(ORDINAL_INDEX);
+ ASSERT_FALSE(col_meta.has_precision());
+ ASSERT_FALSE(col_meta.has_frac());
+ setup_parsed_segment_footer({col_meta});
+
+ const auto& parsed_col_meta = _mock_segment->get_footer()->columns(0);
+ EXPECT_EQ(parsed_col_meta.precision(), 20);
+ EXPECT_EQ(parsed_col_meta.frac(), 6);
+
+ std::shared_ptr<ColumnReader> reader;
+ Status status = _cache->get_column_reader(col_uid, &reader, &_stats);
+ ASSERT_TRUE(status.ok()) << status.to_string();
+ ASSERT_NE(reader, nullptr);
+
+ const auto* data_type =
+ assert_cast<const
DataTypeDecimalV2*>(reader->get_vec_data_type().get());
+ ASSERT_NE(data_type, nullptr);
+ EXPECT_EQ(data_type->get_original_precision(), 20);
+ EXPECT_EQ(data_type->get_original_scale(), 6);
+ EXPECT_EQ(data_type->get_precision(), 27);
+ EXPECT_EQ(data_type->get_scale(), 9);
+}
+
+TEST_F(ColumnReaderCacheTest,
FillMissingDecimalV2PrecisionForComplexTypeFromTabletSchema) {
+ constexpr int32_t col_uid = 11;
+ setup_column_uid_mapping(col_uid, 0);
+ auto& tablet_column =
_mock_segment->tablet_schema()->mutable_column_by_uid(col_uid);
+ tablet_column.set_type(FieldType::OLAP_FIELD_TYPE_ARRAY);
+ TabletColumn item_column;
+ item_column.set_type(FieldType::OLAP_FIELD_TYPE_DECIMAL);
+ item_column.set_precision(18);
+ item_column.set_frac(4);
+ tablet_column.add_sub_column(item_column);
+
+ ColumnMetaPB col_meta;
+ col_meta.set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_ARRAY));
+ col_meta.set_unique_id(col_uid);
+
+ auto* item_meta = col_meta.add_children_columns();
+
item_meta->set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_DECIMAL));
+
item_meta->set_encoding(get_v2_default_encoding(static_cast<FieldType>(item_meta->type())));
+ item_meta->set_num_rows(1000);
+ item_meta->mutable_indexes()->Add()->set_type(ORDINAL_INDEX);
+ ASSERT_FALSE(item_meta->has_precision());
+ ASSERT_FALSE(item_meta->has_frac());
+
+ auto* offset_meta = col_meta.add_children_columns();
+
offset_meta->set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_UNSIGNED_BIGINT));
+
offset_meta->set_encoding(get_v2_default_encoding(static_cast<FieldType>(offset_meta->type())));
+ offset_meta->set_num_rows(1000);
+ offset_meta->mutable_indexes()->Add()->set_type(ORDINAL_INDEX);
+ setup_parsed_segment_footer({col_meta});
+
+ const auto& parsed_item_meta =
_mock_segment->get_footer()->columns(0).children_columns(0);
+ EXPECT_EQ(parsed_item_meta.precision(), 18);
+ EXPECT_EQ(parsed_item_meta.frac(), 4);
+
+ std::shared_ptr<ColumnReader> reader;
+ Status status = _cache->get_column_reader(col_uid, &reader, &_stats);
+ ASSERT_TRUE(status.ok()) << status.to_string();
+ ASSERT_NE(reader, nullptr);
+
+ auto data_type = reader->get_vec_data_type();
+ ASSERT_NE(data_type, nullptr);
+ EXPECT_EQ(data_type->get_primitive_type(), TYPE_ARRAY);
+ const auto* array_type = assert_cast<const
DataTypeArray*>(data_type.get());
+ const auto* nested_type_nullable =
+ assert_cast<const
DataTypeNullable*>(array_type->get_nested_type().get());
+ ASSERT_NE(nested_type_nullable, nullptr);
+ const auto* nested_type =
+ assert_cast<const
DataTypeDecimalV2*>(nested_type_nullable->get_nested_type().get());
+ ASSERT_NE(nested_type, nullptr);
+ EXPECT_EQ(nested_type->get_original_precision(), 18);
+ EXPECT_EQ(nested_type->get_original_scale(), 4);
+ EXPECT_EQ(nested_type->get_precision(), 27);
+ EXPECT_EQ(nested_type->get_scale(), 9);
+}
+TEST_F(ColumnReaderCacheTest, FillMissingDecimalV3PrecisionFromTabletSchema) {
+ constexpr int32_t col_uid = 10;
+ setup_column_uid_mapping(col_uid, 0);
+ auto& tablet_column =
_mock_segment->tablet_schema()->mutable_column_by_uid(col_uid);
+ tablet_column.set_type(FieldType::OLAP_FIELD_TYPE_DECIMAL128I);
+ tablet_column.set_precision(20);
+ tablet_column.set_frac(6);
+
+ ColumnMetaPB col_meta;
+
col_meta.set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_DECIMAL128I));
+ col_meta.set_unique_id(col_uid);
+
col_meta.set_encoding(get_v2_default_encoding(static_cast<FieldType>(col_meta.type())));
+ col_meta.mutable_indexes()->Add()->set_type(ORDINAL_INDEX);
+ ASSERT_FALSE(col_meta.has_precision());
+ ASSERT_FALSE(col_meta.has_frac());
+ setup_parsed_segment_footer({col_meta});
+
+ const auto& parsed_col_meta = _mock_segment->get_footer()->columns(0);
+ EXPECT_EQ(parsed_col_meta.precision(), 20);
+ EXPECT_EQ(parsed_col_meta.frac(), 6);
+
+ std::shared_ptr<ColumnReader> reader;
+ Status status = _cache->get_column_reader(col_uid, &reader, &_stats);
+ ASSERT_TRUE(status.ok()) << status.to_string();
+ ASSERT_NE(reader, nullptr);
+
+ auto data_type = reader->get_vec_data_type();
+ ASSERT_NE(data_type, nullptr);
+ EXPECT_EQ(data_type->get_primitive_type(), TYPE_DECIMAL128I);
+ EXPECT_EQ(data_type->get_precision(), 20);
+ EXPECT_EQ(data_type->get_scale(), 6);
+}
+
+TEST_F(ColumnReaderCacheTest,
FillMissingDecimalV3PrecisionForComplexTypeFromTabletSchema) {
+ constexpr int32_t col_uid = 11;
+ setup_column_uid_mapping(col_uid, 0);
+ auto& tablet_column =
_mock_segment->tablet_schema()->mutable_column_by_uid(col_uid);
+ tablet_column.set_type(FieldType::OLAP_FIELD_TYPE_ARRAY);
+ TabletColumn item_column;
+ item_column.set_type(FieldType::OLAP_FIELD_TYPE_DECIMAL64);
+ item_column.set_precision(18);
+ item_column.set_frac(4);
+ tablet_column.add_sub_column(item_column);
+
+ ColumnMetaPB col_meta;
+ col_meta.set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_ARRAY));
+ col_meta.set_unique_id(col_uid);
+
+ auto* item_meta = col_meta.add_children_columns();
+
item_meta->set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_DECIMAL64));
+
item_meta->set_encoding(get_v2_default_encoding(static_cast<FieldType>(item_meta->type())));
+ item_meta->set_num_rows(1000);
+ item_meta->mutable_indexes()->Add()->set_type(ORDINAL_INDEX);
+ ASSERT_FALSE(item_meta->has_precision());
+ ASSERT_FALSE(item_meta->has_frac());
+
+ auto* offset_meta = col_meta.add_children_columns();
+
offset_meta->set_type(static_cast<int32_t>(FieldType::OLAP_FIELD_TYPE_UNSIGNED_BIGINT));
+
offset_meta->set_encoding(get_v2_default_encoding(static_cast<FieldType>(offset_meta->type())));
+ offset_meta->set_num_rows(1000);
+ offset_meta->mutable_indexes()->Add()->set_type(ORDINAL_INDEX);
+ setup_parsed_segment_footer({col_meta});
+
+ const auto& parsed_item_meta =
_mock_segment->get_footer()->columns(0).children_columns(0);
+ EXPECT_EQ(parsed_item_meta.precision(), 18);
+ EXPECT_EQ(parsed_item_meta.frac(), 4);
+
+ std::shared_ptr<ColumnReader> reader;
+ Status status = _cache->get_column_reader(col_uid, &reader, &_stats);
+ ASSERT_TRUE(status.ok()) << status.to_string();
+ ASSERT_NE(reader, nullptr);
+
+ auto data_type = reader->get_vec_data_type();
+ ASSERT_NE(data_type, nullptr);
+ EXPECT_EQ(data_type->get_primitive_type(), TYPE_ARRAY);
+ const auto* array_type = assert_cast<const
DataTypeArray*>(data_type.get());
+ auto nested_type = array_type->get_nested_type();
+ ASSERT_NE(nested_type, nullptr);
+ EXPECT_EQ(nested_type->get_primitive_type(), TYPE_DECIMAL64);
+ EXPECT_EQ(nested_type->get_precision(), 18);
+ EXPECT_EQ(nested_type->get_scale(), 4);
+}
+
+} // namespace doris::segment_v2
diff --git a/be/test/storage/segment/mock/mock_segment.h
b/be/test/storage/segment/mock/mock_segment.h
index 6c715a53dee..1c145a55827 100644
--- a/be/test/storage/segment/mock/mock_segment.h
+++ b/be/test/storage/segment/mock/mock_segment.h
@@ -21,7 +21,9 @@
#include <gmock/gmock.h>
#include <memory>
+#include <string>
#include <unordered_map>
+#include <utility>
#include "io/fs/file_reader.h"
#include "storage/segment/segment.h"
@@ -49,13 +51,24 @@ public:
// Helper methods for test setup
void add_column_uid_mapping(int32_t col_uid, int32_t footer_ordinal) {
- _tablet_schema->_cols.push_back(std::make_shared<TabletColumn>());
- _tablet_schema->_cols.back()->set_unique_id(col_uid);
- _tablet_schema->_field_uniqueid_to_index[col_uid] = footer_ordinal;
+ (void)footer_ordinal;
+ TabletColumn column;
+ column.set_unique_id(col_uid);
+ column.set_name(std::to_string(col_uid));
+ _tablet_schema->append_column(column);
}
void set_footer(std::shared_ptr<SegmentFooterPB> footer) { _footer =
footer; }
+ void set_file_reader_for_test(io::FileReaderSPtr file_reader) {
+ _file_reader = std::move(file_reader);
+ }
+
+ Status parse_footer_for_test(std::shared_ptr<SegmentFooterPB>& footer,
+ OlapReaderStatistics* stats = nullptr) {
+ return _parse_footer(footer, stats);
+ }
+
void set_column_raw_data_bytes(int32_t uid, uint64_t bytes) {
_column_uid_to_raw_bytes[uid] = bytes;
}
@@ -68,4 +81,4 @@ public:
friend class ColumnReaderCache;
};
-} // namespace doris::segment_v2
\ No newline at end of file
+} // namespace doris::segment_v2
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
