Skip to content

Commit 144d982

Browse files
authored
fix(storage): restore cluster stats pages for rollback compatibility (#20192)
1 parent 311d2f6 commit 144d982

2 files changed

Lines changed: 74 additions & 42 deletions

File tree

src/query/storages/common/table_meta/src/meta/v2/statistics.rs

Lines changed: 68 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,15 @@ pub struct ClusterStatistics {
6969
)]
7070
pub max: Vec<Scalar>,
7171
pub level: i32,
72+
73+
// Page pruning has been removed, but this field must remain in the persisted wire format so
74+
// binaries released before its removal can still deserialize newly written metadata.
75+
#[serde(
76+
default,
77+
serialize_with = "serialize_index_scalar_option_vec",
78+
deserialize_with = "deserialize_index_scalar_option_vec"
79+
)]
80+
pub pages: Option<Vec<Scalar>>,
7281
}
7382

7483
/// Spatial statistics for geometry columns.
@@ -334,6 +343,7 @@ impl ClusterStatistics {
334343
min,
335344
max,
336345
level,
346+
pages: None,
337347
}
338348
}
339349

@@ -383,6 +393,7 @@ impl ClusterStatistics {
383393
min,
384394
max,
385395
level: v0.level,
396+
pages: None,
386397
})
387398
}
388399
}
@@ -506,6 +517,40 @@ where D: serde::Deserializer<'de> {
506517
.collect::<Result<Vec<_>, _>>()
507518
}
508519

520+
fn serialize_index_scalar_option_vec<S>(
521+
scalars: &Option<Vec<Scalar>>,
522+
serializer: S,
523+
) -> Result<S::Ok, S::Error>
524+
where
525+
S: serde::Serializer,
526+
{
527+
match scalars {
528+
Some(scalars) => serialize_index_scalar_vec(scalars, serializer),
529+
None => serializer.serialize_none(),
530+
}
531+
}
532+
533+
fn deserialize_index_scalar_option_vec<'de, D>(
534+
deserializer: D,
535+
) -> Result<Option<Vec<Scalar>>, D::Error>
536+
where D: serde::Deserializer<'de> {
537+
<Option<Vec<IndexScalar>> as serde::Deserialize>::deserialize(deserializer)?
538+
.map(|index_scalars| {
539+
index_scalars
540+
.into_iter()
541+
.map(|index_scalar| {
542+
Scalar::try_from(index_scalar).map_err(|e| {
543+
D::Error::custom(format!(
544+
"Failed to convert IndexScalar to Scalar: {:?}",
545+
e
546+
))
547+
})
548+
})
549+
.collect::<Result<Vec<_>, _>>()
550+
})
551+
.transpose()
552+
}
553+
509554
/// Deserializes the `col_stats` field of the `BlockMeta` and `Statistics` struct.
510555
///
511556
/// This function is designed to handle legacy `ColumnStatistics` items that incorrectly
@@ -579,59 +624,43 @@ impl<'de> serde::de::Visitor<'de> for ColStatsVisitor {
579624

580625
#[cfg(test)]
581626
mod tests {
582-
use databend_common_expression::converts::meta::IndexScalar;
583-
584627
use super::*;
585628

586-
#[derive(serde::Serialize, serde::Deserialize)]
587-
struct LegacyClusterStatistics {
629+
#[derive(serde::Serialize)]
630+
struct ClusterStatisticsWithoutPages {
588631
cluster_key_id: u32,
589-
min: Vec<IndexScalar>,
590-
max: Vec<IndexScalar>,
632+
#[serde(serialize_with = "serialize_index_scalar_vec")]
633+
min: Vec<Scalar>,
634+
#[serde(serialize_with = "serialize_index_scalar_vec")]
635+
max: Vec<Scalar>,
591636
level: i32,
592-
pages: Option<Vec<IndexScalar>>,
593-
}
594-
595-
fn legacy_stats() -> LegacyClusterStatistics {
596-
LegacyClusterStatistics {
597-
cluster_key_id: 7,
598-
min: vec![IndexScalar::Number(1_i64.into())],
599-
max: vec![IndexScalar::Number(9_i64.into())],
600-
level: 2,
601-
pages: Some(vec![IndexScalar::Tuple(vec![IndexScalar::Number(
602-
1_i64.into(),
603-
)])]),
604-
}
605-
}
606-
607-
#[test]
608-
fn reads_legacy_cluster_statistics_with_pages() {
609-
let bytes = rmp_serde::to_vec_named(&legacy_stats()).unwrap();
610-
let stats: ClusterStatistics = rmp_serde::from_slice(&bytes).unwrap();
611-
612-
assert_eq!(
613-
stats,
614-
ClusterStatistics::new(
615-
7,
616-
vec![Scalar::Number(1_i64.into())],
617-
vec![Scalar::Number(9_i64.into())],
618-
2,
619-
)
620-
);
621637
}
622638

623639
#[test]
624-
fn legacy_reader_accepts_cluster_statistics_without_pages() {
640+
fn writes_pages_for_legacy_readers() {
625641
let stats = ClusterStatistics::new(
626642
7,
627643
vec![Scalar::Number(1_i64.into())],
628644
vec![Scalar::Number(9_i64.into())],
629645
2,
630646
);
631647
let bytes = rmp_serde::to_vec_named(&stats).unwrap();
632-
let legacy: LegacyClusterStatistics = rmp_serde::from_slice(&bytes).unwrap();
648+
let value: serde_json::Value = rmp_serde::from_slice(&bytes).unwrap();
649+
650+
assert_eq!(value.get("pages"), Some(&serde_json::Value::Null));
651+
}
652+
653+
#[test]
654+
fn reads_cluster_statistics_written_without_pages() {
655+
let stats = ClusterStatisticsWithoutPages {
656+
cluster_key_id: 7,
657+
min: vec![Scalar::Number(1_i64.into())],
658+
max: vec![Scalar::Number(9_i64.into())],
659+
level: 2,
660+
};
661+
let bytes = rmp_serde::to_vec_named(&stats).unwrap();
662+
let decoded: ClusterStatistics = rmp_serde::from_slice(&bytes).unwrap();
633663

634-
assert_eq!(legacy.cluster_key_id, 7);
635-
assert_eq!(legacy.pages, None);
664+
assert_eq!(decoded, ClusterStatistics::new(7, stats.min, stats.max, 2));
636665
}
637666
}

src/query/storages/common/table_meta/src/meta/v3/frozen/block_meta.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ pub struct ClusterStatistics {
141141
pub max: Vec<LegacyScalar>,
142142
pub level: i32,
143143

144-
// Removed from the current `ClusterStatistics`, but retained here to decode
145-
// legacy bincode segment metadata without changing its positional layout.
144+
// Page pruning is no longer used, but this field is retained to decode legacy bincode segment
145+
// metadata without changing its positional layout.
146146
pub pages: Option<Vec<LegacyScalar>>,
147147
}
148148

@@ -153,17 +153,20 @@ impl From<ClusterStatistics> for crate::meta::ClusterStatistics {
153153
min,
154154
max,
155155
level,
156-
pages: _,
156+
pages,
157157
} = value;
158158
let min: Vec<_> = min.into_iter().map(Scalar::from).collect();
159159

160160
let max: Vec<_> = max.into_iter().map(Scalar::from).collect();
161161

162+
let pages = pages.map(|pages| pages.into_iter().map(Scalar::from).collect());
163+
162164
Self {
163165
cluster_key_id,
164166
min,
165167
max,
166168
level,
169+
pages,
167170
}
168171
}
169172
}

0 commit comments

Comments
 (0)