Skip to content

Commit 2258cad

Browse files
committed
fix(query): relax decimal domain assertion to scale and precision
`statistics_to_domain` asserted that the persisted decimal scalar variant matches the `data_kind()` derived from the current schema size. That is not an invariant: statistics built from an external Parquet column or page index carry the variant implied by the Parquet physical type, so a `Decimal128` scalar can legitimately describe a precision-9 (`Decimal64`-kind) size. The assertion broke `parquet_rs::prune_pages::test_basic`, `prune_pages::test_decimal_in_list` and `prune_row_groups::test_various_rg_scenarios`. Keep only the invariants that hold - equal scale and a persisted precision no wider than the current schema precision - and let the domain keep following the scalar variant, as it did before precision widening became metadata-only.
1 parent e9c1121 commit 2258cad

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

src/query/storages/common/index/src/range_index.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,12 +325,19 @@ pub fn statistics_to_domain(mut stats: Vec<&ColumnStatistics>, data_type: &DataT
325325
// A metadata-only decimal precision widening leaves persisted block and
326326
// segment statistics tagged with the previous precision. The scale and raw
327327
// value are unchanged, so build the domain with the current schema size.
328+
//
329+
// Only the scale and the precision direction are checked here. The scalar
330+
// variant is deliberately not compared against `size.data_kind()`: statistics
331+
// built from an external Parquet column/page index carry the variant implied
332+
// by the Parquet physical type, so a `Decimal128` scalar can legitimately
333+
// describe a `Decimal64`-kind size. The domain therefore keeps following the
334+
// scalar variant, as it did before precision widening became metadata-only.
328335
let min_decimal = min.as_decimal().unwrap();
329336
let max_decimal = max.as_decimal().unwrap();
330337
debug_assert_eq!(min_decimal.scale(), size.scale());
331338
debug_assert_eq!(max_decimal.scale(), size.scale());
332-
debug_assert_eq!(min_decimal.data_kind(), size.data_kind());
333-
debug_assert_eq!(max_decimal.data_kind(), size.data_kind());
339+
debug_assert!(min_decimal.size().precision() <= size.precision());
340+
debug_assert!(max_decimal.size().precision() <= size.precision());
334341
let domain = match min_decimal {
335342
DecimalScalar::Decimal64(_, _) => {
336343
let domain = SimpleDomain {

src/query/storages/common/index/tests/it/range_pruner.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,31 @@ fn test_decimal_statistics_use_current_schema_size() {
146146
}
147147
}
148148

149+
// Statistics built from an external Parquet column/page index carry the scalar variant
150+
// implied by the Parquet physical type, which may be wider than the variant that the
151+
// schema size alone would imply (e.g. `Decimal128` for a precision-9 size).
152+
#[test]
153+
fn test_decimal_statistics_allow_wider_scalar_variant_than_schema_size() {
154+
let size = DecimalSize::new(9, 2).unwrap();
155+
let statistics = ColumnStatistics {
156+
min: Scalar::Decimal(DecimalScalar::Decimal128(100, size)),
157+
max: Scalar::Decimal(DecimalScalar::Decimal128(600, size)),
158+
null_count: 0,
159+
in_memory_size: 0,
160+
distinct_of_values: None,
161+
};
162+
163+
let domain = statistics_to_domain(vec![&statistics], &DataType::Decimal(size));
164+
match domain {
165+
Domain::Decimal(DecimalDomain::Decimal128(domain, domain_size)) => {
166+
assert_eq!(domain.min, 100);
167+
assert_eq!(domain.max, 600);
168+
assert_eq!(domain_size, size);
169+
}
170+
domain => panic!("unexpected domain: {domain:?}"),
171+
}
172+
}
173+
149174
#[test]
150175
fn test_range_index_prunes_nullable_integer_column_eq_numeric_string_literal() {
151176
fn n(n: i32) -> Scalar {

0 commit comments

Comments
 (0)