From c82ac552c32d1bf8a137077682bb59137ae1d966 Mon Sep 17 00:00:00 2001 From: Paul Masurel Date: Fri, 28 Aug 2026 15:33:03 +0200 Subject: [PATCH] Make aggregation column block accessor private Move ColumnBlockAccessor out of tantivy-columnar and into the aggregation implementation. Keep its behavior and tests intact while removing it from the public columnar API. --- columnar/src/lib.rs | 2 -- src/aggregation/agg_data.rs | 6 ++-- .../src => src/aggregation}/block_accessor.rs | 36 ++++++++++--------- src/aggregation/bucket/multi_terms/mod.rs | 6 ++-- .../bucket/term_agg/term_histogram.rs | 4 +-- src/aggregation/mod.rs | 3 ++ 6 files changed, 30 insertions(+), 27 deletions(-) rename {columnar/src => src/aggregation}/block_accessor.rs (94%) diff --git a/columnar/src/lib.rs b/columnar/src/lib.rs index 2db89ef28..1da8d9604 100644 --- a/columnar/src/lib.rs +++ b/columnar/src/lib.rs @@ -24,7 +24,6 @@ extern crate more_asserts; use std::fmt::Display; use std::io; -mod block_accessor; mod column; pub mod column_index; pub mod column_values; @@ -35,7 +34,6 @@ mod iterable; pub(crate) mod utils; mod value; -pub use block_accessor::ColumnBlockAccessor; pub use column::{BytesColumn, Column, StrColumn}; pub use column_index::ColumnIndex; pub use column_values::{ diff --git a/src/aggregation/agg_data.rs b/src/aggregation/agg_data.rs index 0027c17d4..1e0b3423b 100644 --- a/src/aggregation/agg_data.rs +++ b/src/aggregation/agg_data.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use columnar::{Column, ColumnBlockAccessor, ColumnType, StrColumn}; +use columnar::{Column, ColumnType, StrColumn}; use common::BitSet; use rustc_hash::FxHashSet; use serde::Serialize; @@ -29,7 +29,7 @@ use crate::aggregation::metric::{ use crate::aggregation::segment_agg_result::{ GenericSegmentAggregationResultsCollector, SegmentAggregationCollector, }; -use crate::aggregation::{f64_to_fastfield_u64, AggContextParams, Key}; +use crate::aggregation::{f64_to_fastfield_u64, AggContextParams, ColumnBlockAccessor, Key}; use crate::{SegmentOrdinal, SegmentReader}; #[derive(Default)] @@ -39,7 +39,7 @@ pub struct AggregationsSegmentCtx { /// Request data for each aggregation type. pub per_request: PerRequestAggSegCtx, pub context: AggContextParams, - pub column_block_accessor: ColumnBlockAccessor, + pub(crate) column_block_accessor: ColumnBlockAccessor, } impl AggregationsSegmentCtx { diff --git a/columnar/src/block_accessor.rs b/src/aggregation/block_accessor.rs similarity index 94% rename from columnar/src/block_accessor.rs rename to src/aggregation/block_accessor.rs index ec6abe7e2..94a1cf335 100644 --- a/columnar/src/block_accessor.rs +++ b/src/aggregation/block_accessor.rs @@ -1,9 +1,11 @@ use std::cmp::Ordering; -use crate::{Column, DocId, RowId}; +use columnar::{Column, RowId}; + +use crate::DocId; #[derive(Debug, Default, Clone)] -pub struct ColumnBlockAccessor { +pub(crate) struct ColumnBlockAccessor { val_cache: Vec, docid_cache: Vec, missing_docids_cache: Vec, @@ -14,7 +16,7 @@ impl ColumnBlockAccessor { #[inline] - pub fn fetch_block<'a>(&'a mut self, docs: &'a [u32], accessor: &Column) { + pub(crate) fn fetch_block<'a>(&'a mut self, docs: &'a [u32], accessor: &Column) { self.fetch_block_with_is_full(docs, accessor, accessor.index.get_cardinality().is_full()); } @@ -23,7 +25,7 @@ impl /// checked once at construction). `is_full` must equal /// `accessor.index.get_cardinality().is_full()`. #[inline] - pub fn fetch_block_with_is_full<'a>( + pub(crate) fn fetch_block_with_is_full<'a>( &'a mut self, docs: &'a [u32], accessor: &Column, @@ -59,7 +61,7 @@ impl /// Fetches a block and appends `missing_opt` for documents without a value. #[inline] - pub fn fetch_block_with_missing( + pub(crate) fn fetch_block_with_missing( &mut self, docs: &[u32], accessor: &Column, @@ -72,7 +74,7 @@ impl /// true, the missing entries are inserted in document order instead of appended as a second /// run. #[inline] - pub fn fetch_block_with_missing_ordered( + pub(crate) fn fetch_block_with_missing_ordered( &mut self, docs: &[u32], accessor: &Column, @@ -142,7 +144,7 @@ impl /// This is necessary for correct document counting in aggregations, /// where multi-valued fields can produce duplicate entries that inflate counts. #[inline] - pub fn fetch_block_with_missing_unique_per_doc( + pub(crate) fn fetch_block_with_missing_unique_per_doc( &mut self, docs: &[u32], accessor: &Column, @@ -211,18 +213,18 @@ impl /// Returns the values fetched by the last `fetch_block*` call. #[inline] - pub fn values(&self) -> &[T] { + pub(crate) fn values(&self) -> &[T] { &self.val_cache } /// Returns the document IDs corresponding to [`Self::values`] for a non-full column. #[inline] - pub fn docids(&self) -> &[DocId] { + pub(crate) fn docids(&self) -> &[DocId] { &self.docid_cache } #[inline] - pub fn iter_vals(&self) -> impl ExactSizeIterator + '_ { + pub(crate) fn iter_vals(&self) -> impl ExactSizeIterator + '_ { self.val_cache.iter().cloned() } @@ -233,7 +235,7 @@ impl /// /// The docs is used if the column is full (each docs has exactly one value), otherwise the /// internal docid vec is used for the iterator, which e.g. may contain duplicate docs. - pub fn iter_docid_vals<'a>( + pub(crate) fn iter_docid_vals<'a>( &'a self, docs: &'a [u32], accessor: &Column, @@ -339,9 +341,9 @@ mod tests { #[test] fn test_fetch_block_with_missing_ordered() { - use crate::column_index::{ColumnIndex, OptionalIndex}; - use crate::column_values::{ - ALL_U64_CODEC_TYPES, serialize_and_load_u64_based_column_values, + use columnar::column_index::{ColumnIndex, OptionalIndex}; + use columnar::column_values::{ + serialize_and_load_u64_based_column_values, ALL_U64_CODEC_TYPES, }; let vals = [10u64, 40, 70]; @@ -430,9 +432,9 @@ mod tests { #[test] fn test_fetch_block_contiguous_and_gather_match() { - use crate::column_index::ColumnIndex; - use crate::column_values::{ - ALL_U64_CODEC_TYPES, serialize_and_load_u64_based_column_values, + use columnar::column_index::ColumnIndex; + use columnar::column_values::{ + serialize_and_load_u64_based_column_values, ALL_U64_CODEC_TYPES, }; let vals: Vec = (0..200u64).map(|i| i * 7 + 3).collect(); diff --git a/src/aggregation/bucket/multi_terms/mod.rs b/src/aggregation/bucket/multi_terms/mod.rs index 60fd3b5f5..d9e8e5912 100644 --- a/src/aggregation/bucket/multi_terms/mod.rs +++ b/src/aggregation/bucket/multi_terms/mod.rs @@ -4,8 +4,8 @@ use std::sync::Arc; use columnar::column_values::CompactSpaceU64Accessor; use columnar::{ - Column, ColumnBlockAccessor, ColumnType, Dictionary, MonotonicallyMappableToU128, - MonotonicallyMappableToU64, NumericalValue, StrColumn, + Column, ColumnType, Dictionary, MonotonicallyMappableToU128, MonotonicallyMappableToU64, + NumericalValue, StrColumn, }; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; @@ -30,7 +30,7 @@ use crate::aggregation::intermediate_agg_result::{ IntermediateKey, IntermediateTermBucketEntry, PruneMode, }; use crate::aggregation::segment_agg_result::{BucketIdProvider, SegmentAggregationCollector}; -use crate::aggregation::{f64_to_fastfield_u64, format_date, BucketId, Key}; +use crate::aggregation::{f64_to_fastfield_u64, format_date, BucketId, ColumnBlockAccessor, Key}; use crate::{DocId, TantivyError}; /// Multi-terms aggregation: one bucket per unique combination of values across N term fields. diff --git a/src/aggregation/bucket/term_agg/term_histogram.rs b/src/aggregation/bucket/term_agg/term_histogram.rs index 05578f780..368532181 100644 --- a/src/aggregation/bucket/term_agg/term_histogram.rs +++ b/src/aggregation/bucket/term_agg/term_histogram.rs @@ -6,7 +6,7 @@ use std::fmt::Debug; -use columnar::{Column, ColumnBlockAccessor, ColumnType}; +use columnar::{Column, ColumnType}; use super::{ Bucket, SegmentTermCollector, TermsAggReqData, VecTermBuckets, MAX_NUM_BUCKETS_FOR_COUNT_LANES, @@ -22,7 +22,7 @@ use crate::aggregation::intermediate_agg_result::{ IntermediateAggregationResult, IntermediateAggregationResults, }; use crate::aggregation::segment_agg_result::{BucketIdProvider, SegmentAggregationCollector}; -use crate::aggregation::{f64_from_fastfield_u64, BucketId}; +use crate::aggregation::{f64_from_fastfield_u64, BucketId, ColumnBlockAccessor}; /// Maximum number of physical counters in the fused flat grid. Above this the grid would be too /// large/cache-unfriendly, so we fall back to the general buffered path. Count lanes are included diff --git a/src/aggregation/mod.rs b/src/aggregation/mod.rs index a36bbc2c2..4c1820189 100644 --- a/src/aggregation/mod.rs +++ b/src/aggregation/mod.rs @@ -132,6 +132,7 @@ mod agg_data; mod agg_limits; pub mod agg_req; pub mod agg_result; +mod block_accessor; pub mod bucket; pub(crate) mod buffered_sub_aggs; mod collector; @@ -143,6 +144,8 @@ pub mod metric; mod segment_agg_result; use std::fmt::Display; +pub(crate) use block_accessor::ColumnBlockAccessor; + #[cfg(test)] mod agg_tests;