From 3ddca31292dcf5e49bf091f64bffd29ec0bf5b1e Mon Sep 17 00:00:00 2001
From: Pascal Seitz <pascal.seitz@datadoghq.com>
Date: Fri, 12 Dec 2025 17:38:37 +0800
Subject: [PATCH] simplify fetch block in column_block_accessor

---
 columnar/src/block_accessor.rs           | 10 +++++++++-
 src/aggregation/bucket/term_agg.rs       | 16 +++++-----------
 src/aggregation/metric/cardinality.rs    | 14 +++++---------
 src/aggregation/metric/extended_stats.rs | 12 +++---------
 src/aggregation/metric/percentiles.rs    | 14 +++++---------
 src/aggregation/metric/stats.rs          | 14 +++++---------
 6 files changed, 32 insertions(+), 48 deletions(-)

diff --git a/columnar/src/block_accessor.rs b/columnar/src/block_accessor.rs
index 6bd24ba3b..9926553a8 100644
--- a/columnar/src/block_accessor.rs
+++ b/columnar/src/block_accessor.rs
@@ -29,12 +29,20 @@ impl<T: PartialOrd + Copy + std::fmt::Debug + Send + Sync + 'static + Default>
         }
     }
     #[inline]
-    pub fn fetch_block_with_missing(&mut self, docs: &[u32], accessor: &Column<T>, missing: T) {
+    pub fn fetch_block_with_missing(
+        &mut self,
+        docs: &[u32],
+        accessor: &Column<T>,
+        missing: Option<T>,
+    ) {
         self.fetch_block(docs, accessor);
         // no missing values
         if accessor.index.get_cardinality().is_full() {
             return;
         }
+        let Some(missing) = missing else {
+            return;
+        };
 
         // We can compare docid_cache length with docs to find missing docs
         // For multi value columns we can't rely on the length and always need to scan
diff --git a/src/aggregation/bucket/term_agg.rs b/src/aggregation/bucket/term_agg.rs
index 4d569ddad..5cf39746c 100644
--- a/src/aggregation/bucket/term_agg.rs
+++ b/src/aggregation/bucket/term_agg.rs
@@ -803,17 +803,11 @@ impl<TermMap: TermAggregationMap, const LOWCARD: bool> SegmentAggregationCollect
 
         let req_data = &mut self.terms_req_data;
 
-        if let Some(missing) = req_data.missing_value_for_accessor {
-            agg_data.column_block_accessor.fetch_block_with_missing(
-                docs,
-                &req_data.accessor,
-                missing,
-            );
-        } else {
-            agg_data
-                .column_block_accessor
-                .fetch_block(docs, &req_data.accessor);
-        }
+        agg_data.column_block_accessor.fetch_block_with_missing(
+            docs,
+            &req_data.accessor,
+            req_data.missing_value_for_accessor,
+        );
 
         if let Some(sub_agg) = &mut self.sub_agg {
             let term_buckets = &mut self.parent_buckets[parent_bucket_id as usize];
diff --git a/src/aggregation/metric/cardinality.rs b/src/aggregation/metric/cardinality.rs
index 9336e04cb..c184848d8 100644
--- a/src/aggregation/metric/cardinality.rs
+++ b/src/aggregation/metric/cardinality.rs
@@ -236,15 +236,11 @@ impl SegmentCardinalityCollector {
         docs: &[crate::DocId],
         agg_data: &mut AggregationsSegmentCtx,
     ) {
-        if let Some(missing) = self.missing_value_for_accessor {
-            agg_data
-                .column_block_accessor
-                .fetch_block_with_missing(docs, &self.accessor, missing);
-        } else {
-            agg_data
-                .column_block_accessor
-                .fetch_block(docs, &self.accessor);
-        }
+        agg_data.column_block_accessor.fetch_block_with_missing(
+            docs,
+            &self.accessor,
+            self.missing_value_for_accessor,
+        );
     }
 }
 
diff --git a/src/aggregation/metric/extended_stats.rs b/src/aggregation/metric/extended_stats.rs
index af159e787..e71426790 100644
--- a/src/aggregation/metric/extended_stats.rs
+++ b/src/aggregation/metric/extended_stats.rs
@@ -373,15 +373,9 @@ impl SegmentAggregationCollector for SegmentExtendedStatsCollector {
     ) -> crate::Result<()> {
         let mut extended_stats = self.buckets[parent_bucket_id as usize].clone();
 
-        if let Some(missing) = self.missing.as_ref() {
-            agg_data
-                .column_block_accessor
-                .fetch_block_with_missing(docs, &self.accessor, *missing);
-        } else {
-            agg_data
-                .column_block_accessor
-                .fetch_block(docs, &self.accessor);
-        }
+        agg_data
+            .column_block_accessor
+            .fetch_block_with_missing(docs, &self.accessor, self.missing);
         for val in agg_data.column_block_accessor.iter_vals() {
             let val1 = f64_from_fastfield_u64(val, self.field_type);
             extended_stats.collect(val1);
diff --git a/src/aggregation/metric/percentiles.rs b/src/aggregation/metric/percentiles.rs
index 3f9a9ca1f..ff9de45f1 100644
--- a/src/aggregation/metric/percentiles.rs
+++ b/src/aggregation/metric/percentiles.rs
@@ -282,15 +282,11 @@ impl SegmentAggregationCollector for SegmentPercentilesCollector {
         agg_data: &mut AggregationsSegmentCtx,
     ) -> crate::Result<()> {
         let percentiles = &mut self.buckets[parent_bucket_id as usize];
-        if let Some(missing) = self.missing_u64.as_ref() {
-            agg_data
-                .column_block_accessor
-                .fetch_block_with_missing(docs, &self.accessor, *missing);
-        } else {
-            agg_data
-                .column_block_accessor
-                .fetch_block(docs, &self.accessor);
-        }
+        agg_data.column_block_accessor.fetch_block_with_missing(
+            docs,
+            &self.accessor,
+            self.missing_u64,
+        );
 
         for val in agg_data.column_block_accessor.iter_vals() {
             let val1 = f64_from_fastfield_u64(val, self.field_type);
diff --git a/src/aggregation/metric/stats.rs b/src/aggregation/metric/stats.rs
index 5ec1b0184..c43a6a259 100644
--- a/src/aggregation/metric/stats.rs
+++ b/src/aggregation/metric/stats.rs
@@ -285,15 +285,11 @@ impl<const COLUMN_TYPE_ID: u8> SegmentAggregationCollector
 
             return Ok(());
         }
-        if let Some(missing) = self.missing_u64.as_ref() {
-            agg_data
-                .column_block_accessor
-                .fetch_block_with_missing(docs, &self.accessor, *missing);
-        } else {
-            agg_data
-                .column_block_accessor
-                .fetch_block(docs, &self.accessor);
-        }
+        agg_data.column_block_accessor.fetch_block_with_missing(
+            docs,
+            &self.accessor,
+            self.missing_u64,
+        );
         collect_stats::<COLUMN_TYPE_ID>(
             &mut self.buckets[parent_bucket_id as usize],
             agg_data.column_block_accessor.iter_vals(),