Skip to content

[fix](be) Fix missing decimal precision and scale for legacy decimal data #65419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jacktengg wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[fix](be) Fix missing decimal precision and scale for legacy decimal data #65419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4283283
[fix](be) Fix missing decimal precision and scale for legacy decimal …
jacktengg Jul 9, 2026
3d6edca
improve
jacktengg Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions be/src/core/data_type/data_type_decimal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,15 @@ class DataTypeDecimal final : public IDataType, public DecimalScaleInfo<T> {

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) {}

Expand Down
19 changes: 2 additions & 17 deletions be/src/core/data_type/data_type_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion be/src/storage/index/inverted/inverted_index_iterator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 8 additions & 0 deletions be/src/storage/olap_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
2 changes: 1 addition & 1 deletion be/src/storage/segment/column_reader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {

@yiguolei yiguolei Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个变动的原因是？

@jacktengg jacktengg Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

删除重复代码is_numeric_type。

try {
index_reader = BkdIndexReader::create_shared(index_meta, index_file_reader);
} catch (const CLuceneError& e) {
Expand Down
47 changes: 47 additions & 0 deletions be/src/storage/segment/segment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand Down
26 changes: 0 additions & 26 deletions be/src/storage/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 { \
Expand Down
170 changes: 0 additions & 170 deletions be/test/core/data_type/data_type_decimal_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading
Loading