[fix](be) Handle legacy DecimalV2 segments with missing precision/frac#63569
Open
jacktengg wants to merge 1 commit into
Open
[fix](be) Handle legacy DecimalV2 segments with missing precision/frac#63569jacktengg wants to merge 1 commit into
jacktengg wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review result: no blocking issues found.
Critical checkpoint conclusions:
- Goal and proof: The PR targets legacy DecimalV2 metadata where precision/frac are absent or nonpositive, and the added unit test covers TabletColumn and segment ColumnMetaPB creation paths plus a modern decimal(26,6) case.
- Scope and clarity: The implementation is small and localized to the OLAP_FIELD_TYPE_DECIMAL factory path used by tablet/segment metadata.
- Concurrency and lifecycle: No new concurrency, locking, static initialization, or lifecycle-sensitive behavior is introduced.
- Compatibility: The change preserves modern persisted DecimalV2 precision/scale and restores compatibility for legacy metadata by using the existing UINT32_MAX unknown-original sentinel.
- Parallel paths: The affected segment/tablet metadata paths both flow through _create_primitive_data_type; unrelated runtime/thrift/PColumnMeta paths are not part of the legacy segment failure described.
- Error handling and data correctness: Invalid positive precision/scale still goes through DataTypeDecimalV2 validation; legacy missing metadata now maps to in-memory DecimalV2(27,9) with original precision/scale fallback as intended.
- Tests: Focused BE unit coverage was added. I did not run tests in this review environment.
- Observability, persistence, transaction, memory: Not applicable; no new persistence format, transaction path, memory ownership, config, or observability behavior is introduced.
User focus points: No additional user-provided review focus was supplied.
Contributor
TPC-H: Total hot run time: 31765 ms |
Contributor
TPC-DS: Total hot run time: 172905 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run p0 |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
May 25, 2026
…recision/frac (apache#63569) Pick: apache#63569 Problem Summary: After upgrading from 3.1.4 to 4.0.4, queries on DecimalV2 columns fail with: [INTERNAL_ERROR]meet invalid precision: real_precision=0, max_decimal_precision=27, min_decimal_precision=1 Root cause: Segments written by Doris < 2.1.0 (before apache#26572) do not persist precision/frac in ColumnMetaPB; they default to 0 when read back. Since apache#35222, DataTypeFactory passes those raw values as original_precision/original_scale into DataTypeDecimalV2, which calls check_type_precision(0) and throws. 3.1.4 was unaffected because the old code hardcoded (27, 9). Fix: In _create_primitive_data_type for OLAP_FIELD_TYPE_DECIMAL, when precision is not positive (legacy segment), pass UINT32_MAX to DataTypeDecimalV2 to signal that the original precision/scale are unknown. DecimalScaleInfo<TYPE_DECIMALV2> already treats UINT32_MAX as 'unknown' and falls back to the in-memory (27, 9) representation in get_original_precision()/get_original_scale(). Fix "meet invalid precision: real_precision=0" when querying DecimalV2 columns on segments written by Doris versions older than 2.1.0. - Test: Unit Test - Added DataTypeDecimalTest.create_decimalv2_from_legacy_tablet_column covering: legacy TabletColumn (unset precision/frac), modern TabletColumn with decimal(26,6), and legacy segment_v2::ColumnMetaPB with default-0 precision/frac. - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Open
16 tasks
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
May 25, 2026
…recision/frac (apache#63569) Pick: apache#63569 Problem Summary: After upgrading from 3.1.4 to 4.0.4, queries on DecimalV2 columns fail with: [INTERNAL_ERROR]meet invalid precision: real_precision=0, max_decimal_precision=27, min_decimal_precision=1 Root cause: Segments written by Doris < 2.1.0 (before apache#26572) do not persist precision/frac in ColumnMetaPB; they default to 0 when read back. Since apache#35222, DataTypeFactory passes those raw values as original_precision/original_scale into DataTypeDecimalV2, which calls check_type_precision(0) and throws. 3.1.4 was unaffected because the old code hardcoded (27, 9). Fix: In _create_primitive_data_type for OLAP_FIELD_TYPE_DECIMAL, when precision is not positive (legacy segment), pass UINT32_MAX to DataTypeDecimalV2 to signal that the original precision/scale are unknown. DecimalScaleInfo<TYPE_DECIMALV2> already treats UINT32_MAX as 'unknown' and falls back to the in-memory (27, 9) representation in get_original_precision()/get_original_scale(). Fix "meet invalid precision: real_precision=0" when querying DecimalV2 columns on segments written by Doris versions older than 2.1.0. - Test: Unit Test - Added DataTypeDecimalTest.create_decimalv2_from_legacy_tablet_column covering: legacy TabletColumn (unset precision/frac), modern TabletColumn with decimal(26,6), and legacy segment_v2::ColumnMetaPB with default-0 precision/frac. - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jacktengg
force-pushed
the
260525-fix-decv2
branch
2 times, most recently
from
ba47a4a to
7aa502e
Compare
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
I found one blocking correctness regression in the PR.
Critical checkpoint conclusions:
- Goal/test: The PR addresses legacy DecimalV2 segments with default-0 precision/frac and adds focused BE unit coverage for DecimalV2, including nested complex-type metadata. The goal is mostly covered, but the implementation changes behavior for other primitive types in the same segment metadata path.
- Scope/focus: The PR is small and focused in the actual GitHub diff.
- Concurrency/lifecycle/config/compatibility: No new concurrency, special lifecycle, or config items. The change is a storage metadata compatibility fix; compatibility handling is needed and currently too broad.
- Parallel paths: The TabletColumn path already passes -1/default sentinel values and is not the failing path; the ColumnMetaPB path is the relevant read path.
- Tests: DecimalV2 tests were added, but they miss non-decimal primitive types that also flow through the modified branch, especially DateTimeV2/TIMESTAMPTZ scale handling.
- Observability/transactions/data writes: Not applicable.
- Performance: No material concern.
- User focus: No additional user-provided review focus was supplied.
Please restrict the UINT32_MAX fallback to OLAP_FIELD_TYPE_DECIMAL/DecimalV2 (or otherwise preserve pcolumn.frac() for types that use it as scale) and add coverage for a non-decimal primitive with scale metadata.
| auto meta_precision = pcolumn.precision(); | ||
| UInt32 orig_precision = | ||
| meta_precision > 0 ? static_cast<UInt32>(meta_precision) : UINT32_MAX; | ||
| UInt32 orig_scale = meta_precision > 0 ? static_cast<UInt32>(pcolumn.frac()) : UINT32_MAX; |
Contributor
There was a problem hiding this comment.
This fallback is applied to every primitive ColumnMetaPB, not just legacy DecimalV2. For example, DATETIMEV2/TIMESTAMPTZ reach this same branch and _create_primitive_data_type() uses the scale argument in create_datetimev2(scale). Their precision() is normally unset/default, while frac() carries the scale; with this change meta_precision <= 0 rewrites that scale to UINT32_MAX, so reading a DATETIMEV2 segment can throw scale_value 4294967295 > 6. Please gate the UINT32_MAX sentinel on OLAP_FIELD_TYPE_DECIMAL only (or otherwise preserve pcolumn.frac() for non-DecimalV2 types), and add a regression test for a scaled DATETIMEV2/TIMESTAMPTZ ColumnMetaPB.
Contributor
TPC-H: Total hot run time: 31678 ms |
Issue Number: close #xxx
Problem Summary: After upgrading from 3.1.4 to 4.0.4, queries on
DecimalV2 columns fail with:
[INTERNAL_ERROR]meet invalid precision: real_precision=0,
max_decimal_precision=27, min_decimal_precision=1
Root cause: Segments written by Doris < 2.1.0 (before apache#26572) do not
persist precision/frac in ColumnMetaPB; they default to 0 when read
back. Since apache#35222, DataTypeFactory passes those raw values as
original_precision/original_scale into DataTypeDecimalV2, which calls
check_type_precision(0) and throws. 3.1.4 was unaffected because the
old code hardcoded (27, 9).
Fix: In _create_primitive_data_type for OLAP_FIELD_TYPE_DECIMAL, when
precision is not positive (legacy segment), pass UINT32_MAX to
DataTypeDecimalV2 to signal that the original precision/scale are
unknown. DecimalScaleInfo<TYPE_DECIMALV2> already treats UINT32_MAX as
'unknown' and falls back to the in-memory (27, 9) representation in
get_original_precision()/get_original_scale().
Fix "meet invalid precision: real_precision=0" when querying DecimalV2
columns on segments written by Doris versions older than 2.1.0.
- Test: Unit Test
- Added DataTypeDecimalTest.create_decimalv2_from_legacy_tablet_column
covering: legacy TabletColumn (unset precision/frac), modern
TabletColumn with decimal(26,6), and legacy segment_v2::ColumnMetaPB
with default-0 precision/frac.
PS:
error callstack:
```
I20260525 16:41:28.185905 1945583 pipeline_fragment_context.cpp:177] PipelineFragmentContext::cancel|query_id=18982a6c19ee4166-ae3ba44bce094741|fragment_id=1|reason=[INTERNAL_ERROR]meet invalid precision: real_precision=0, max_decimal_precision=27, min_decimal_precision=1
0# doris::get_stack_trace(int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/util/stack_util.cpp:59
1# doris::Exception::Exception(int, std::basic_string_view<char, std::char_traits<char> > const&, bool) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/common/exception.cpp:31
2# doris::Exception::Exception(int, std::basic_string_view<char, std::char_traits<char> > const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/common/exception.h:38
3# doris::Exception::Exception<unsigned int const&, unsigned long>(int, std::basic_string_view<char, std::char_traits<char> > const&, unsigned int const&, unsigned long&&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/common/exception.h:44
4# doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>::check_type_precision(unsigned int) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/data_types/data_type_decimal.h:280
5# doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>::DataTypeDecimal(unsigned int, unsigned int, unsigned int, unsigned int) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/data_types/data_type_decimal.h:144
6# void std::_Construct<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>, int, int, int&, int&>(doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>*, int&&, int&&, int&, int&) at /mnt/disk2/tengjianping/local/ldb_toolchain/bin/../lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/stl_construct.h:134
7# std::_Sp_counted_ptr_inplace<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<int, int, int&, int&>(std::allocator<void>, int&&, int&&, int&, int&) at /mnt/disk2/tengjianping/local/ldb_toolchain/bin/../lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/alloc_traits.h:805
8# std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>, std::allocator<void>, int, int, int&, int&>(doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>*&, std::_Sp_alloc_shared_tag<std::allocator<void> >, int&&, int&&, int&, int&) at /mnt/disk2/tengjianping/local/ldb_toolchain/bin/../lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/shared_ptr_base.h:970
9# std::__shared_ptr<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>, int, int, int&, int&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, int&&, int&&, int&, int&) at /mnt/disk2/tengjianping/local/ldb_toolchain/bin/../lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/shared_ptr_base.h:1719
10# std::shared_ptr<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20> >::shared_ptr<std::allocator<void>, int, int, int&, int&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, int&&, int&&, int&, int&) at /mnt/disk2/tengjianping/local/ldb_toolchain/bin/../lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/shared_ptr.h:464
11# std::shared_ptr<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20> > std::make_shared<doris::vectorized::DataTypeDecimal<(doris::PrimitiveType)20>, int, int, int&, int&>(int&&, int&&, int&, int&) at /mnt/disk2/tengjianping/local/ldb_toolchain/bin/../lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/shared_ptr.h:1007
12# doris::vectorized::DataTypeFactory::_create_primitive_data_type(doris::FieldType const&, int, int, int) const at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/data_types/data_type_factory.cpp:196
13# doris::vectorized::DataTypeFactory::create_data_type(doris::segment_v2::ColumnMetaPB const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/data_types/data_type_factory.cpp:394
14# doris::segment_v2::ColumnReader::ColumnReader(doris::segment_v2::ColumnReaderOptions const&, doris::segment_v2::ColumnMetaPB const&, unsigned long, std::shared_ptr<doris::io::FileReader>) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/column_reader.cpp:290
15# doris::segment_v2::ColumnReader::create(doris::segment_v2::ColumnReaderOptions const&, doris::segment_v2::ColumnMetaPB const&, unsigned long, std::shared_ptr<doris::io::FileReader> const&, std::shared_ptr<doris::segment_v2::ColumnReader>*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/column_reader.cpp:242
16# doris::segment_v2::ColumnReaderCache::get_column_reader(int, std::shared_ptr<doris::segment_v2::ColumnReader>*, doris::OlapReaderStatistics*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/column_reader_cache.cpp:138
17# doris::segment_v2::Segment::get_column_reader(int, std::shared_ptr<doris::segment_v2::ColumnReader>*, doris::OlapReaderStatistics*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment.cpp:738
18# doris::segment_v2::Segment::new_column_iterator(doris::TabletColumn const&, std::unique_ptr<doris::segment_v2::ColumnIterator, std::default_delete<doris::segment_v2::ColumnIterator> >*, doris::StorageReadOptions const*, std::unordered_map<int, std::unique_ptr<std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::shared_ptr<doris::segment_v2::SparseColumnCache>, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::shared_ptr<doris::segment_v2::SparseColumnCache> > > >, std::default_delete<std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::shared_ptr<doris::segment_v2::SparseColumnCache>, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::shared_ptr<doris::segment_v2::SparseColumnCache> > > > > >, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<int const, std::unique_ptr<std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::shared_ptr<doris::segment_v2::SparseColumnCache>, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::shared_ptr<doris::segment_v2::SparseColumnCache> > > >, std::default_delete<std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::shared_ptr<doris::segment_v2::SparseColumnCache>, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::shared_ptr<doris::segment_v2::SparseColumnCache> > > > > > > > > const*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment.cpp:679
19# doris::segment_v2::SegmentIterator::_init_return_column_iterators() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment_iterator.cpp:1391
20# doris::segment_v2::SegmentIterator::init_iterators() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment_iterator.cpp:396
21# doris::segment_v2::SegmentIterator::_init_impl(doris::StorageReadOptions const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment_iterator.cpp:372
22# doris::segment_v2::SegmentIterator::init(doris::StorageReadOptions const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment_iterator.cpp:290
23# doris::segment_v2::Segment::new_iterator(std::shared_ptr<doris::Schema const>, doris::StorageReadOptions const&, std::unique_ptr<doris::RowwiseIterator, std::default_delete<doris::RowwiseIterator> >*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/segment.cpp:0
24# doris::segment_v2::LazyInitSegmentIterator::init(doris::StorageReadOptions const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/segment_v2/lazy_init_segment_iterator.cpp:48
25# doris::BetaRowsetReader::_init_iterator() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/beta_rowset_reader.cpp:330
26# doris::BetaRowsetReader::_init_iterator_once()::$_0::operator()() const at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/beta_rowset_reader.cpp:295
27# doris::Status doris::DorisCallOnce<doris::Status>::call<doris::BetaRowsetReader::_init_iterator_once()::$_0>(doris::BetaRowsetReader::_init_iterator_once()::$_0) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/util/once.h:92
28# doris::BetaRowsetReader::_init_iterator_once() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/beta_rowset_reader.cpp:295
29# doris::Status doris::BetaRowsetReader::_next_batch<doris::vectorized::Block>(doris::vectorized::Block*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/beta_rowset_reader.h:103
30# doris::BetaRowsetReader::next_batch(doris::vectorized::Block*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/olap/rowset/beta_rowset_reader.h:56
31# doris::vectorized::VCollectIterator::Level0Iterator::_refresh() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/vcollect_iterator.h:0
32# doris::vectorized::VCollectIterator::Level0Iterator::refresh_current_row() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/vcollect_iterator.cpp:526
33# doris::vectorized::VCollectIterator::Level0Iterator::ensure_first_row_ref() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/vcollect_iterator.cpp:497
34# doris::vectorized::VCollectIterator::Level1Iterator::ensure_first_row_ref() at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/vcollect_iterator.cpp:715
35# doris::vectorized::VCollectIterator::build_heap(std::vector<std::shared_ptr<doris::RowsetReader>, std::allocator<std::shared_ptr<doris::RowsetReader> > >&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/vcollect_iterator.cpp:190
36# doris::vectorized::BlockReader::_init_collect_iter(doris::TabletReader::ReaderParams const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/block_reader.cpp:152
37# doris::vectorized::BlockReader::init(doris::TabletReader::ReaderParams const&) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/olap/block_reader.cpp:226
38# doris::vectorized::OlapScanner::_open_impl(doris::RuntimeState*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/exec/scan/olap_scanner.cpp:266
39# doris::vectorized::Scanner::open(doris::RuntimeState*) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/exec/scan/scanner.h:84
40# doris::vectorized::ScannerScheduler::_scanner_scan(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>) at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/exec/scan/scanner_scheduler.cpp:187
41# doris::vectorized::ScannerScheduler::submit(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>)::$_0::operator()() const::{lambda()apache#1}::operator()() const::{lambda()apache#1}::operator()() const at /mnt/disk2/tengjianping/doris-4.0-dev/be/src/vec/exec/scan/scanner_scheduler.cpp:76
```
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jacktengg
force-pushed
the
260525-fix-decv2
branch
from
7aa502e to
aee0b66
Compare
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review summary: no additional blocking issues found in the current head.
Critical checkpoint conclusions:
- Goal/test: The change targets legacy DecimalV2 segment metadata where ColumnMetaPB precision/frac default to 0. The current code fixes the segment-read path and adds unit coverage for legacy, modern, nested, and AggState DecimalV2 metadata.
- Scope/focus: The implementation is small and localized to DataTypeFactory segment ColumnMetaPB handling plus focused tests.
- Concurrency/lifecycle: No new shared state, locks, static initialization, or lifecycle-sensitive ownership paths are introduced.
- Compatibility/data correctness: The fallback is now gated to OLAP_FIELD_TYPE_DECIMAL, preserving frac for non-decimal primitive types such as DATETIMEV2/TIMESTAMPTZ. DecimalV2 unknown original precision/scale uses the existing UINT32_MAX sentinel semantics.
- Parallel paths: TabletColumn already represents unset legacy values as -1, which converts to the existing unknown sentinel; the fixed path is the ColumnMetaPB segment path where absent fields default to 0.
- Tests: Added BE unit tests cover the reported path and recursive complex-type construction. I did not run the unit test in this review environment.
- Observability/config/transactions/persistence/security: Not applicable; no new config, persistence format, logging, transaction, auth, or network behavior is introduced.
- User focus: No additional user-provided review focus was specified.
Existing review context: the previously raised concern about applying UINT32_MAX to all primitive ColumnMetaPB types is not repeated here because the current head gates that behavior to DecimalV2 only.
Contributor
Author
|
run buildall |
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
May 25, 2026
…recision/frac (apache#63569) Pick: apache#63569 Problem Summary: After upgrading from 3.1.4 to 4.0.4, queries on DecimalV2 columns fail with: [INTERNAL_ERROR]meet invalid precision: real_precision=0, max_decimal_precision=27, min_decimal_precision=1 Root cause: Segments written by Doris < 2.1.0 (before apache#26572) do not persist precision/frac in ColumnMetaPB; they default to 0 when read back. Since apache#35222, DataTypeFactory passes those raw values as original_precision/original_scale into DataTypeDecimalV2, which calls check_type_precision(0) and throws. 3.1.4 was unaffected because the old code hardcoded (27, 9). Fix: In _create_primitive_data_type for OLAP_FIELD_TYPE_DECIMAL, when precision is not positive (legacy segment), pass UINT32_MAX to DataTypeDecimalV2 to signal that the original precision/scale are unknown. DecimalScaleInfo<TYPE_DECIMALV2> already treats UINT32_MAX as 'unknown' and falls back to the in-memory (27, 9) representation in get_original_precision()/get_original_scale(). Fix "meet invalid precision: real_precision=0" when querying DecimalV2 columns on segments written by Doris versions older than 2.1.0. - Test: Unit Test - Added DataTypeDecimalTest.create_decimalv2_from_legacy_tablet_column covering: legacy TabletColumn (unset precision/frac), modern TabletColumn with decimal(26,6), and legacy segment_v2::ColumnMetaPB with default-0 precision/frac. - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
TPC-DS: Total hot run time: 174435 ms |
Contributor
TPC-H: Total hot run time: 32175 ms |
Contributor
TPC-DS: Total hot run time: 173804 ms |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31499 ms |
Contributor
TPC-DS: Total hot run time: 173113 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Problem Summary: After upgrading from 3.1.4 to 4.0.4, queries on DecimalV2 columns fail with:
Root cause: Segments written by Doris < 2.1.0 (before #26572) do not persist precision/frac in ColumnMetaPB; they default to 0 when read back. Since #35222, DataTypeFactory passes those raw values as original_precision/original_scale into DataTypeDecimalV2, which calls check_type_precision(0) and throws. 3.1.4 was unaffected because the old code hardcoded (27, 9).
Fix: In _create_primitive_data_type for OLAP_FIELD_TYPE_DECIMAL, when precision is not positive (legacy segment), pass UINT32_MAX to DataTypeDecimalV2 to signal that the original precision/scale are unknown. DecimalScaleInfo<TYPE_DECIMALV2> already treats UINT32_MAX as 'unknown' and falls back to the in-memory (27, 9) representation in get_original_precision()/get_original_scale().
Release note
Fix "meet invalid precision: real_precision=0" when querying DecimalV2 columns on segments written by Doris versions older than 2.1.0.
Check List (For Author)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)