diff --git a/columnar/src/block_accessor.rs b/columnar/src/block_accessor.rs index ed0fe4731..86b40716b 100644 --- a/columnar/src/block_accessor.rs +++ b/columnar/src/block_accessor.rs @@ -56,33 +56,81 @@ impl .get_vals(&self.row_id_cache, &mut self.val_cache); } } + + /// Fetches a block and appends `missing_opt` for documents without a value. #[inline] pub fn fetch_block_with_missing( &mut self, docs: &[u32], accessor: &Column, missing_opt: Option, + ) { + self.fetch_block_with_missing_ordered(docs, accessor, missing_opt, false); + } + + /// Fetches a block and adds `missing_opt` for documents without a value. When `ordered` is + /// true, the missing entries are inserted in document order instead of appended as a second + /// run. + #[inline] + pub fn fetch_block_with_missing_ordered( + &mut self, + docs: &[u32], + accessor: &Column, + missing_opt: Option, + ordered: bool, ) { self.fetch_block(docs, accessor); + let cardinality = accessor.index.get_cardinality(); // no missing values - if accessor.index.get_cardinality().is_full() { + if cardinality.is_full() { return; } let Some(missing) = missing_opt 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 - if accessor.index.get_cardinality().is_multivalue() || docs.len() != self.docid_cache.len() - { - self.missing_docids_cache.clear(); - find_missing_docs(docs, &self.docid_cache, |doc| { - self.missing_docids_cache.push(doc); - self.val_cache.push(missing); - }); + // 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. + let is_multivalue = cardinality.is_multivalue(); + if !is_multivalue && docs.len() == self.docid_cache.len() { + return; + } + + if ordered && !is_multivalue { + // Expand backwards so values that have not moved yet are not overwritten. + let mut end = docs.len(); + self.val_cache.resize(end, missing); + for hit_idx in (0..self.docid_cache.len()).rev() { + let pos = docs[..end].partition_point(|&doc| doc < self.docid_cache[hit_idx]); + self.val_cache[pos + 1..end].fill(missing); + self.val_cache[pos] = self.val_cache[hit_idx]; + end = pos; + } + self.val_cache[..end].fill(missing); + self.docid_cache.clear(); + self.docid_cache.extend_from_slice(docs); + return; + } + + self.missing_docids_cache.clear(); + find_missing_docs(docs, &self.docid_cache, |doc| { + self.missing_docids_cache.push(doc); + }); + + if !ordered { + self.val_cache.resize( + self.val_cache.len() + self.missing_docids_cache.len(), + missing, + ); self.docid_cache .extend_from_slice(&self.missing_docids_cache); + return; + } + + for &doc in &self.missing_docids_cache { + let pos = self.docid_cache.partition_point(|&hit| hit < doc); + self.docid_cache.insert(pos, doc); + self.val_cache.insert(pos, missing); } } @@ -97,10 +145,11 @@ impl docs: &[u32], accessor: &Column, missing: Option, + ordered: bool, ) where T: Ord, { - self.fetch_block_with_missing(docs, accessor, missing); + self.fetch_block_with_missing_ordered(docs, accessor, missing, ordered); if accessor.index.get_cardinality().is_multivalue() { self.dedup_docid_val_pairs(); } @@ -281,6 +330,35 @@ mod tests { assert_eq!(missing_docs, vec![1, 2, 3, 4, 5]); } + #[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, + }; + + let vals = vec![10u64, 40, 70]; + let values = + serialize_and_load_u64_based_column_values::(&&vals[..], &ALL_U64_CODEC_TYPES); + let column = Column { + index: ColumnIndex::Optional(OptionalIndex::for_test(9, &[1, 4, 7])), + values, + }; + let docs = [0, 1, 2, 4, 7, 8]; + let mut accessor = ColumnBlockAccessor::::default(); + + accessor.fetch_block_with_missing_ordered(&docs, &column, Some(99), true); + + assert_eq!( + accessor.iter_vals().collect::>(), + vec![99, 10, 99, 40, 70, 99] + ); + assert_eq!( + accessor.iter_docid_vals(&docs, &column).collect::>(), + vec![(0, 99), (1, 10), (2, 99), (4, 40), (7, 70), (8, 99)] + ); + } + #[test] fn test_dedup_docid_val_pairs_consecutive() { let mut accessor = ColumnBlockAccessor::::default(); diff --git a/src/aggregation/agg_data.rs b/src/aggregation/agg_data.rs index 9db99a629..15ebd98ff 100644 --- a/src/aggregation/agg_data.rs +++ b/src/aggregation/agg_data.rs @@ -769,12 +769,8 @@ fn build_multi_terms_nodes( // columns before injecting the fallback, so a value in another type-specific collector is // not mistaken for a missing field and a genuinely missing document is not counted once // per type. - let missing_choice = prepare_multi_terms_missing( - &columns, - str_dict_column.as_ref(), - field_def.missing.as_ref(), - field_name, - )?; + let missing_choice = + prepare_multi_terms_missing(&columns, field_def.missing.as_ref(), field_name)?; let all_columns: Option]>> = missing_choice.as_ref().map(|_| { Arc::from( columns @@ -787,7 +783,7 @@ fn build_multi_terms_nodes( let mut typed_accessors = Vec::with_capacity(columns.len()); for (column_idx, (column, column_type)) in columns.into_iter().enumerate() { let missing = match (&missing_choice, &all_columns) { - (Some((missing_idx, key_elem)), Some(all_columns)) + (Some((missing_idx, missing_value)), Some(all_columns)) if *missing_idx == column_idx => { Some(MultiTermsMissingAccessor { @@ -796,7 +792,7 @@ fn build_multi_terms_nodes( .missing .clone() .expect("missing choice requires a configured missing value"), - key_elem: *key_elem, + missing_value: *missing_value, }) } _ => None, @@ -861,77 +857,77 @@ fn build_multi_terms_nodes( fn prepare_multi_terms_missing( columns: &[(Column, ColumnType)], - str_dict_column: Option<&StrColumn>, missing: Option<&Key>, field_name: &str, -) -> crate::Result> { - use crate::aggregation::bucket::KeyElem; - +) -> crate::Result> { let Some(missing) = missing else { return Ok(None); }; - if let Key::Str(missing_str) = missing { - let str_column_idx = columns + // Attach string fallbacks to the string column when one exists. A string fallback on any + // other physical type is handled synthetically, just like the special terms missing + // collector. + let column_idx = if matches!(missing, Key::Str(_)) { + columns .iter() - .position(|(_, column_type)| *column_type == ColumnType::Str); - return match (str_column_idx, str_dict_column) { - (Some(column_idx), Some(str_dict_column)) => { - let key_elem = match str_dict_column - .dictionary() - .term_ord(missing_str.as_bytes())? - { - Some(ord) => KeyElem::new(ord), - None => KeyElem::synthetic_missing(), - }; - Ok(Some((column_idx, key_elem))) - } - // No string dictionary is available. Attach the synthetic fallback to one physical - // accessor so only one collector handles a globally missing value. - (Some(column_idx), None) => Ok(Some((column_idx, KeyElem::synthetic_missing()))), - (None, _) => Ok(Some((0, KeyElem::synthetic_missing()))), + .position(|(_, column_type)| *column_type == ColumnType::Str) + .unwrap_or(0) + } else { + // Prefer an exact physical type for numeric missing values, then any numerical type, then + // a string column (which accepts numeric fallbacks through a synthetic value). + let preferred_type = match missing { + Key::F64(_) => ColumnType::F64, + Key::I64(_) => ColumnType::I64, + Key::U64(_) => ColumnType::U64, + Key::Str(_) => unreachable!("handled above"), }; - } - - // Prefer an exact physical type for numeric missing values, then any numerical type. This - // preserves the same lenient coercions used by the terms aggregation. - let preferred_type = match missing { - Key::F64(_) => ColumnType::F64, - Key::I64(_) => ColumnType::I64, - Key::U64(_) => ColumnType::U64, - Key::Str(_) => unreachable!("handled above"), + columns + .iter() + .position(|(_, column_type)| *column_type == preferred_type) + .or_else(|| { + columns + .iter() + .position(|(_, column_type)| column_type.numerical_type().is_some()) + }) + .or_else(|| { + columns + .iter() + .position(|(_, column_type)| *column_type == ColumnType::Str) + }) + .unwrap_or(0) }; - let numeric_column_idx = columns - .iter() - .position(|(_, column_type)| *column_type == preferred_type) - .or_else(|| { - columns - .iter() - .position(|(_, column_type)| column_type.numerical_type().is_some()) - }); - if let Some(column_idx) = numeric_column_idx { - let (column, column_type) = &columns[column_idx]; - return Ok(get_missing_val_as_u64_lenient( - *column_type, - column.max_value(), - missing, - field_name, - )? - .map(|val| (column_idx, KeyElem::new(val)))); + let (column, column_type) = &columns[column_idx]; + if !matches!(missing, Key::Str(_)) && *column_type != ColumnType::Str { + // Validate the same lenient numeric coercions as a terms aggregation. The converted value + // is deliberately ignored: multi_terms uses a collision-free sentinel and resolves it + // back to the request's original missing key. + get_missing_val_as_u64_lenient(*column_type, column.max_value(), missing, field_name)?; } - // A string column accepts numeric missing keys via a synthetic sentinel. Other physical types - // are rejected by `get_missing_val_as_u64_lenient`, matching terms aggregation validation. - let column_idx = columns - .iter() - .position(|(_, column_type)| *column_type == ColumnType::Str) - .unwrap_or(0); - let (column, column_type) = &columns[column_idx]; - Ok( - get_missing_val_as_u64_lenient(*column_type, column.max_value(), missing, field_name)? - .map(|_| (column_idx, KeyElem::synthetic_missing())), - ) + Ok(Some((column_idx, find_missing_sentinel(column)))) +} + +/// Returns a value that cannot collide with a value in `column`. +/// +/// Usually one of the column bounds leaves a free value. Only a column whose bounds span the +/// entire `u64` domain requires the slower scan. +fn find_missing_sentinel(column: &Column) -> u64 { + if let Some(sentinel) = column.max_value().checked_add(1) { + return sentinel; + } + if let Some(sentinel) = column.min_value().checked_sub(1) { + return sentinel; + } + + // TODO: This is an extreme edge case that would be better handled by a collector that does + // not use sentinel missing values. For now, we just scan the column. + let values: FxHashSet = column.values.iter().collect(); + let mut sentinel = 1u64; + while values.contains(&sentinel) { + sentinel += 1; + } + sentinel } fn build_children( diff --git a/src/aggregation/bucket/multi_terms/mod.rs b/src/aggregation/bucket/multi_terms/mod.rs index 3da8c58df..e8ed4202d 100644 --- a/src/aggregation/bucket/multi_terms/mod.rs +++ b/src/aggregation/bucket/multi_terms/mod.rs @@ -30,7 +30,7 @@ use crate::aggregation::intermediate_agg_result::{ IntermediateKey, IntermediateTermBucketEntry, PruneMode, }; use crate::aggregation::segment_agg_result::{BucketIdProvider, SegmentAggregationCollector}; -use crate::aggregation::{format_date, BucketId, Key}; +use crate::aggregation::{f64_to_fastfield_u64, format_date, BucketId, Key}; use crate::TantivyError; /// Multi-terms aggregation: one bucket per unique combination of values across N term fields. @@ -132,8 +132,8 @@ pub struct MultiTermsMissingAccessor { pub all_columns: Arc<[Column]>, /// The user-configured fallback key, used to resolve synthetic missing values. pub key: Key, - /// Precomputed key element to collect when the field is absent from every physical column. - pub key_elem: KeyElem, + /// Collision-free value injected for missing documents. + pub missing_value: u64, } /// One typed accessor for one field in a multi_terms collector. @@ -190,79 +190,53 @@ impl MultiTermsAggReqData { } } -/// One element of a composite key. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] -pub struct KeyElem { - /// Whether this value must resolve directly to the configured missing key. - /// - /// This field comes first so derived ordering keeps synthetic missing values after real - /// column values, matching the old accessor-index sentinel ordering. - synthetic_missing: bool, - /// Raw fast-field u64 value (term ord for Str, `to_u64` encoding for numerics). - pub val: u64, -} - -impl KeyElem { - /// Creates a `KeyElem` for a real column value. - pub fn new(val: u64) -> Self { - Self { - val, - synthetic_missing: false, - } - } - - /// Creates a `KeyElem` that encodes a synthetic missing value. - pub fn synthetic_missing() -> Self { - Self { - val: 0, - synthetic_missing: true, - } - } - - /// Returns `true` if this element encodes a synthetic missing value. - pub fn is_synthetic_missing(self) -> bool { - self.synthetic_missing - } -} - -/// Inline capacity of [`MultiTermsKey`] +/// Inline capacity of [`MultiTermsKey`]. const MULTI_TERMS_KEY_INLINE_CAPACITY: usize = 3; -/// The composite key for one combination of field values, inline-allocated for up to +/// The composite key for one combination of raw fast-field values, inline-allocated for up to /// [`MULTI_TERMS_KEY_INLINE_CAPACITY`] fields. -pub type MultiTermsKey = SmallVec<[KeyElem; MULTI_TERMS_KEY_INLINE_CAPACITY]>; +pub type MultiTermsKey = SmallVec<[u64; MULTI_TERMS_KEY_INLINE_CAPACITY]>; impl AggregationMapKey for MultiTermsKey { fn heap_memory_usage(&self) -> usize { if self.spilled() { - self.capacity() * std::mem::size_of::() + self.capacity() * std::mem::size_of::() } else { 0 } } } -/// One field's candidate `KeyElem`s for the current document. -type FieldValues = SmallVec<[KeyElem; 2]>; +/// One field's candidate values for the current document. +type FieldValues = SmallVec<[u64; 2]>; -/// Fetches one field into the shared block accessor and returns whether its values are dense over -/// `docs`. +/// Returns the missing sentinel that can safely be injected while decoding this physical column. +/// Mixed-type fields require a later union-of-columns existence check instead. +#[inline(always)] +fn block_missing_value(missing: Option<&MultiTermsMissingAccessor>) -> Option { + missing + .filter(|missing| missing.all_columns.len() == 1) + .map(|missing| missing.missing_value) +} + +/// Fetches one field into the shared block accessor and forwards safe missing handling. Ordering +/// is requested only by multi-terms, which needs values from different fields aligned by document. +/// Returns whether every document has a value after decoding. #[inline(always)] fn fetch_field_block( docs: &[crate::DocId], field: &MultiTermsFieldAccessor, + missing: Option<&MultiTermsMissingAccessor>, block_accessor: &mut ColumnBlockAccessor, ) -> bool { - let cardinality = field.column.get_cardinality(); - let is_full = cardinality.is_full(); - - if cardinality.is_multivalue() { - block_accessor.fetch_block_with_missing_unique_per_doc(docs, &field.column, None); - } else { - block_accessor.fetch_block_with_is_full(docs, &field.column, is_full); - } - - is_full + let missing_value = block_missing_value(missing); + block_accessor.fetch_block_with_missing_unique_per_doc( + docs, + &field.column, + missing_value, + true, + ); + field.column.get_cardinality().is_full() || missing_value.is_some() } /// Key operations used by the unified collector. @@ -274,7 +248,7 @@ trait MultiTermsKeyCodec: Clone + Debug + 'static { fn new_key(&self) -> Self::Key; fn clear_key(&self, key: &mut Self::Key); - fn push(&self, key: &mut Self::Key, field_idx: usize, elem: KeyElem); + fn push(&self, key: &mut Self::Key, field_idx: usize, value: u64); fn pop(&self, key: &mut Self::Key, field_idx: usize); /// Pushes one full field's values into every key buffer. Entries belonging to documents @@ -305,9 +279,9 @@ impl MultiTermsKeyCodec for UnpackedKeyCodec { key.clear(); } - fn push(&self, key: &mut Self::Key, field_idx: usize, elem: KeyElem) { + fn push(&self, key: &mut Self::Key, field_idx: usize, value: u64) { debug_assert_eq!(key.len(), field_idx); - key.push(elem); + key.push(value); } fn pop(&self, key: &mut Self::Key, field_idx: usize) { @@ -319,7 +293,7 @@ impl MultiTermsKeyCodec for UnpackedKeyCodec { where I: IntoIterator { for (key, val) in keys.iter_mut().zip(values) { debug_assert!(key.len() <= field_idx); - key.push(KeyElem::new(val)); + key.push(val); } } @@ -422,29 +396,26 @@ fn validate_multi_terms( Ok(()) } -/// Returns the configured missing element if `doc_id` is absent from every physical column for +/// Returns the configured missing sentinel if `doc_id` is absent from every physical column for /// this requested field. `None` means this typed collector branch must drop the document. #[inline] -fn missing_key_elem_for_doc( +fn missing_value_for_doc( missing: Option<&MultiTermsMissingAccessor>, doc_id: crate::DocId, -) -> Option { +) -> Option { let missing = missing?; let field_has_value = missing .all_columns .iter() .any(|column| column.index.has_value(doc_id)); - (!field_has_value).then_some(missing.key_elem) + (!field_has_value).then_some(missing.missing_value) } /// Builds the segment collector for a multi_terms aggregation, validating the request. /// -/// Every request uses the same block-decoding collector. [`compute_packed_u64_layout`] only chooses -/// its key representation: a packed `u64` (plus dense/paged term storage when useful) if the key -/// fits, or [`MultiTermsKey`] in a hash map otherwise. Full and optional columns build one key per -/// surviving document directly. Multivalued columns materialize per-field candidates only when -/// Cartesian key generation is required. Both paths retain packed keys whenever their value ranges -/// fit in 64 bits. +/// Every value, including missing, is represented as a plain `u64` and collected through block +/// decoding. [`compute_packed_u64_layout`] chooses a packed `u64` key when possible and otherwise +/// uses [`MultiTermsKey`]. pub(crate) fn build_segment_multi_terms_collector( req: &mut AggregationsSegmentCtx, node: &AggRefNode, @@ -675,7 +646,12 @@ where // Bulk-decode each full column and build one key per document directly, without // per-document column dispatch or Cartesian recursion. for (field_idx, field) in self.req_data.fields.iter().enumerate() { - fetch_field_block(docs, field, block_accessor); + fetch_field_block( + docs, + field, + self.req_data.missing_accessors[field_idx].as_ref(), + block_accessor, + ); key_codec.push_full_values(keys_buf, field_idx, block_accessor.iter_vals()); } @@ -703,13 +679,16 @@ where break; } - let is_full = fetch_field_block(docs, field, block_accessor); - if is_full { + let missing = self.req_data.missing_accessors[field_idx].as_ref(); + let has_value_per_doc = fetch_field_block(docs, field, missing, block_accessor); + if has_value_per_doc { key_codec.push_full_values(keys_buf, field_idx, block_accessor.iter_vals()); continue; } - let missing = self.req_data.missing_accessors[field_idx].as_ref(); + // Without an injectable sentinel, values remain sorted by document. This path + // covers fields without a fallback and mixed-type fields whose missing value + // must be checked against all physical columns. let mut docids_and_vals = block_accessor .iter_docid_vals(docs, &field.column) .peekable(); @@ -717,18 +696,18 @@ where debug_assert!(docids_and_vals .peek() .is_none_or(|(hit_doc, _)| *hit_doc >= doc_id)); - let column_elem = docids_and_vals + let column_value = docids_and_vals .next_if(|(hit_doc, _)| *hit_doc == doc_id) - .map(|(_, val)| KeyElem::new(val)); + .map(|(_, value)| value); if !valid_docs[doc_idx] { continue; } - if let Some(elem) = - column_elem.or_else(|| missing_key_elem_for_doc(missing, doc_id)) + if let Some(value) = + column_value.or_else(|| missing_value_for_doc(missing, doc_id)) { - key_codec.push(&mut keys_buf[doc_idx], field_idx, elem); + key_codec.push(&mut keys_buf[doc_idx], field_idx, value); } else { valid_docs[doc_idx] = false; } @@ -767,18 +746,18 @@ where } // At least one field is multivalued, so retain every field's candidates until the - // document's Cartesian combinations are generated. Non-full accessors expose sorted - // `(doc_id, value)` pairs and deduplicate repeated values within one document. + // document's Cartesian combinations are generated. let block_accessor = &mut agg_data.column_block_accessor; for (field_idx, field) in self.req_data.fields.iter().enumerate() { - let is_full = fetch_field_block(docs, field, block_accessor); - if is_full { - for (doc_idx, val) in block_accessor.iter_vals().enumerate() { - field_values[doc_idx * num_fields + field_idx].push(KeyElem::new(val)); + let missing = self.req_data.missing_accessors[field_idx].as_ref(); + fetch_field_block(docs, field, missing, block_accessor); + if field.column.get_cardinality().is_full() { + for (doc_idx, value) in block_accessor.iter_vals().enumerate() { + field_values[doc_idx * num_fields + field_idx].push(value); } } else { let mut doc_idx = 0usize; - for (doc_id, val) in block_accessor.iter_docid_vals(docs, &field.column) { + for (doc_id, value) in block_accessor.iter_docid_vals(docs, &field.column) { while docs .get(doc_idx) .is_some_and(|candidate| *candidate < doc_id) @@ -786,21 +765,24 @@ where doc_idx += 1; } debug_assert_eq!(docs.get(doc_idx), Some(&doc_id)); - field_values[doc_idx * num_fields + field_idx].push(KeyElem::new(val)); + field_values[doc_idx * num_fields + field_idx].push(value); } } } - // Missing is field-level, not typed-column-level: only the designated accessor may - // emit it, and only when every physical column for that requested field is absent. + // Mixed-type missing handling cannot be delegated to one physical column: only the + // designated accessor may emit the fallback, and only when every physical column for + // that requested field is absent. for (doc_idx, &doc_id) in docs.iter().enumerate() { for (field_idx, missing) in self.req_data.missing_accessors.iter().enumerate() { + if block_missing_value(missing.as_ref()).is_some() { + continue; + } let values = &mut field_values[doc_idx * num_fields + field_idx]; if values.is_empty() { - if let Some(missing_elem) = - missing_key_elem_for_doc(missing.as_ref(), doc_id) + if let Some(missing_value) = missing_value_for_doc(missing.as_ref(), doc_id) { - values.push(missing_elem); + values.push(missing_value); } } } @@ -882,24 +864,21 @@ where /// Per-field bit layout within a packed `u64` key. /// -/// Field 0 occupies the highest bits, so numeric packed-key order is the same as -/// lexicographic [`KeyElem`] order. Values are offset by `min_value`; synthetic missing values, -/// when needed, reserve the first offset after the real value range. +/// Field 0 occupies the highest bits, so numeric packed-key order is the same as lexicographic raw +/// value order. Values are offset by `min_value`; an injectable missing sentinel is part of that +/// ordinary value range. #[derive(Clone, Copy, Debug)] struct FieldPack { shift: u32, mask: u64, min_value: u64, max_offset: u64, - synthetic_offset: Option, } /// Computes a packed-`u64` layout regardless of column cardinality. /// -/// `None` now means only that the lossless encoded key needs more than 64 bits. Optional and -/// multivalued columns remain eligible. A non-full field's configured missing value is included in -/// its value range; synthetic missing values reserve a distinct offset so they can be resolved -/// after collection. +/// `None` means that the lossless encoded key needs more than 64 bits. Optional and multivalued +/// columns remain eligible, and a reachable missing sentinel is included in its field's range. fn compute_packed_u64_layout( fields: &[MultiTermsFieldAccessor], missing_accessors: &[Option], @@ -913,35 +892,28 @@ fn compute_packed_u64_layout( for (field, missing) in fields.iter().zip(missing_accessors.iter()) { let mut min_value = field.column.min_value(); let mut max_value = field.column.max_value(); - let mut synthetic_offset = None; // Full columns cannot emit missing for any document in this segment, so do not widen their // packed domain for an unreachable fallback. if !field.column.get_cardinality().is_full() { if let Some(missing) = missing { - if missing.key_elem.is_synthetic_missing() { - let real_range = max_value - min_value; - synthetic_offset = Some(real_range.checked_add(1)?); - } else { - min_value = min_value.min(missing.key_elem.val); - max_value = max_value.max(missing.key_elem.val); - } + min_value = min_value.min(missing.missing_value); + max_value = max_value.max(missing.missing_value); } } - let real_range = max_value - min_value; - let max_offset = synthetic_offset.unwrap_or(real_range).max(real_range); + let max_offset = max_value - min_value; let width = 64 - max_offset.leading_zeros(); total_width = total_width.checked_add(width)?; if total_width > 64 { return None; } - field_layouts.push((width, min_value, max_offset, synthetic_offset)); + field_layouts.push((width, min_value, max_offset)); } let mut packs = Vec::with_capacity(fields.len()); let mut shift = 0u32; - for &(width, min_value, max_offset, synthetic_offset) in field_layouts.iter().rev() { + for &(width, min_value, max_offset) in field_layouts.iter().rev() { let mask = if width == 64 { u64::MAX } else if width == 0 { @@ -954,7 +926,6 @@ fn compute_packed_u64_layout( mask, min_value, max_offset, - synthetic_offset, }); shift += width; } @@ -994,16 +965,11 @@ impl MultiTermsKeyCodec for PackedU64KeyCodec { *key = 0; } - fn push(&self, key: &mut Self::Key, field_idx: usize, elem: KeyElem) { + fn push(&self, key: &mut Self::Key, field_idx: usize, value: u64) { let pack = self.packs[field_idx]; - let offset = if elem.is_synthetic_missing() { - pack.synthetic_offset - .expect("packed synthetic missing value has a reserved offset") - } else { - elem.val - .checked_sub(pack.min_value) - .expect("packed value is not below the field minimum") - }; + let offset = value + .checked_sub(pack.min_value) + .expect("packed value is not below the field minimum"); debug_assert!(offset <= pack.max_offset); *key |= shift_packed_bits(offset, pack.shift); } @@ -1035,12 +1001,7 @@ impl MultiTermsKeyCodec for PackedU64KeyCodec { .zip(req_data.missing_accessors.iter()) .map(|((pack, field), missing)| { let offset = key.checked_shr(pack.shift).unwrap_or(0) & pack.mask; - let elem = if pack.synthetic_offset == Some(offset) { - KeyElem::synthetic_missing() - } else { - KeyElem::new(offset + pack.min_value) - }; - resolve_key_elem(elem, field, missing.as_ref()) + resolve_key_value(offset + pack.min_value, field, missing.as_ref()) }) .collect() } @@ -1234,7 +1195,7 @@ where }) } -/// Resolve a composite key (one `KeyElem` per field) to a `Vec`. +/// Resolve a composite key (one raw fast-field value per field) to intermediate keys. fn resolve_multi_terms_key( key: &MultiTermsKey, req_data: &MultiTermsAggReqData, @@ -1242,33 +1203,61 @@ fn resolve_multi_terms_key( key.iter() .zip(req_data.fields.iter()) .zip(req_data.missing_accessors.iter()) - .map(|((elem, field_acc), missing)| resolve_key_elem(*elem, field_acc, missing.as_ref())) + .map(|((value, field_acc), missing)| resolve_key_value(*value, field_acc, missing.as_ref())) .collect() } -/// Resolve one [`KeyElem`] for one field to an [`IntermediateKey`]. -fn resolve_key_elem( - elem: KeyElem, +/// Resolve one raw fast-field value, recognizing the collision-free missing sentinel first. +fn resolve_key_value( + value: u64, field_acc: &MultiTermsFieldAccessor, missing: Option<&MultiTermsMissingAccessor>, ) -> crate::Result { - if elem.is_synthetic_missing() { - let missing = missing.ok_or_else(|| { - TantivyError::AggregationError(crate::aggregation::AggregationError::InternalError( - "multi_terms synthetic missing key has no missing accessor".to_string(), - )) - })?; - return Ok(IntermediateKey::from(missing.key.clone())); + if let Some(missing) = missing.filter(|missing| missing.missing_value == value) { + return resolve_missing_key(missing, field_acc); } resolve_column_value( - elem.val, + value, &field_acc.column_type, &field_acc.str_dict_column, &field_acc.column, ) } +/// Resolve the request's missing key using the selected column's lenient numeric coercion. String +/// fallbacks (and numeric fallbacks attached to string columns) keep their request representation. +fn resolve_missing_key( + missing: &MultiTermsMissingAccessor, + field_acc: &MultiTermsFieldAccessor, +) -> crate::Result { + let numeric_value = match missing.key { + Key::F64(value) => Some(value), + Key::I64(value) => Some(value as f64), + Key::U64(value) => Some(value as f64), + Key::Str(_) => None, + }; + if field_acc.column_type.numerical_type().is_some() { + if let Some(value) = numeric_value { + let encoded = f64_to_fastfield_u64(value, &field_acc.column_type).ok_or_else(|| { + TantivyError::AggregationError(crate::aggregation::AggregationError::InternalError( + format!( + "could not encode multi_terms missing value for column type {:?}", + field_acc.column_type + ), + )) + })?; + return resolve_column_value( + encoded, + &field_acc.column_type, + &field_acc.str_dict_column, + &field_acc.column, + ); + } + } + Ok(IntermediateKey::from(missing.key.clone())) +} + /// Convert a raw u64 from a specific column type to an [`IntermediateKey`]. /// /// Mirrors the logic in `composite/collector.rs:resolve_term` but emits @@ -1832,14 +1821,14 @@ mod tests { for i in 0..64u64 { let inline_key: MultiTermsKey = (0..MULTI_TERMS_KEY_INLINE_CAPACITY) - .map(|field_idx| KeyElem::new(i + field_idx as u64)) + .map(|field_idx| i + field_idx as u64) .collect(); let spilled_key: MultiTermsKey = (0..num_spilled_fields) - .map(|field_idx| KeyElem::new(i + field_idx as u64)) + .map(|field_idx| i + field_idx as u64) .collect(); assert!(!inline_key.spilled()); assert!(spilled_key.spilled()); - expected_spilled_bytes += spilled_key.capacity() * std::mem::size_of::(); + expected_spilled_bytes += spilled_key.capacity() * std::mem::size_of::(); inline_map.term_entry(inline_key, &mut bucket_id_provider); spilled_map.term_entry(spilled_key, &mut bucket_id_provider); @@ -2218,6 +2207,42 @@ mod tests { Ok(()) } + #[test] + fn test_multi_terms_missing_fast_path_noncontiguous_docs() -> crate::Result<()> { + let index = build_two_field_index( + &[ + ("even", Some("A")), + ("odd", Some("B")), + ("even", None), + ("odd", Some("B")), + ("even", Some("A")), + ], + &[], + false, + )?; + + let agg_req: Aggregations = serde_json::from_value(json!({ + "mt": { + "multi_terms": { + "terms": [ + {"field": "genre"}, + {"field": "product", "missing": "MISSING"} + ] + } + } + }))?; + let res = exec_request_with_query(agg_req, &index, Some(("genre", "even")))?; + let buckets = res["mt"]["buckets"].as_array().unwrap(); + assert_eq!(buckets.len(), 2, "unexpected {buckets:?}"); + assert!(buckets + .iter() + .any(|bucket| bucket["key_as_string"] == "even|A" && bucket["doc_count"] == 2)); + assert!(buckets + .iter() + .any(|bucket| bucket["key_as_string"] == "even|MISSING" && bucket["doc_count"] == 1)); + Ok(()) + } + #[test] fn test_multi_terms_wide_fields_use_unpacked_key() -> crate::Result<()> { // `score` alone needs the full 64 bits (max_value close to u64::MAX), so combined @@ -2308,8 +2333,8 @@ mod tests { } #[test] - fn test_multi_terms_unpacked_key_handles_multivalue_missing_and_sub_aggregation( - ) -> crate::Result<()> { + fn test_multi_terms_u64_max_handles_multivalue_missing_and_sub_aggregation() -> crate::Result<()> + { let mut schema_builder = Schema::builder(); let tag_field = schema_builder.add_text_field("tag", STRING | FAST); let big_field = schema_builder diff --git a/src/aggregation/bucket/term_agg/mod.rs b/src/aggregation/bucket/term_agg/mod.rs index 2983502c8..14d14f6d8 100644 --- a/src/aggregation/bucket/term_agg/mod.rs +++ b/src/aggregation/bucket/term_agg/mod.rs @@ -1061,6 +1061,7 @@ impl SegmentAggregationCollector docs, &req_data.accessor, req_data.missing_value_for_accessor, + false, ); if let Some(sub_agg) = &mut self.sub_agg {