diff --git a/lib/common/common/src/condition_checker.rs b/lib/common/common/src/condition_checker.rs index 6f3c23fea0..8a80157bfb 100644 --- a/lib/common/common/src/condition_checker.rs +++ b/lib/common/common/src/condition_checker.rs @@ -1,3 +1,5 @@ +use std::marker::PhantomData; + use crate::types::PointOffsetType; /// A check that tests whether points satisfy a condition. @@ -17,10 +19,23 @@ pub trait ConditionChecker { } } -impl Result> ConditionChecker for F { - type Error = E; +/// A checker that ignores the point and always returns the same value. +pub struct ConstantConditionChecker(bool, PhantomData); - fn check(&self, point_id: PointOffsetType) -> Result { - self(point_id) +impl ConstantConditionChecker { + pub const MATCH_NONE: Self = Self(false, PhantomData); + + pub const MATCH_ALL: Self = Self(true, PhantomData); + + pub const fn new(value: bool) -> Self { + ConstantConditionChecker(value, PhantomData) + } +} + +impl ConditionChecker for ConstantConditionChecker { + type Error = E; + + fn check(&self, _point_id: PointOffsetType) -> Result { + Ok(self.0) } } diff --git a/lib/segment/src/index/field_index/bool_index/read_ops.rs b/lib/segment/src/index/field_index/bool_index/read_ops.rs index 679e028297..85664ca7b2 100644 --- a/lib/segment/src/index/field_index/bool_index/read_ops.rs +++ b/lib/segment/src/index/field_index/bool_index/read_ops.rs @@ -18,6 +18,7 @@ use std::path::PathBuf; +use common::condition_checker::ConditionChecker; use common::counter::hardware_accumulator::HwMeasurementAcc; use common::counter::hardware_counter::HardwareCounterCell; use common::counter::iterator_hw_measurement::HwMeasurementIteratorExt; @@ -25,7 +26,7 @@ use common::types::PointOffsetType; use serde_json::Value; use crate::common::flags::roaring_flags::RoaringFlagsRead; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::common::utils::MultiValue; use crate::index::field_index::{CardinalityEstimation, PayloadBlockCondition, PrimaryCondition}; use crate::index::payload_config::StorageType; @@ -287,12 +288,10 @@ pub(super) fn condition_checker<'a, N: BoolIndexRead>( match cond_match { Match::Value(MatchValue { value: ValueVariants::Bool(is_true), - }) => { - let is_true = *is_true; - Some(Box::new(move |point_id: PointOffsetType| { - Ok(idx.check_values_any(point_id, is_true)) - })) - } + }) => Some(Box::new(BoolConditionChecker { + idx, + is_true: *is_true, + })), Match::Value(MatchValue { value: ValueVariants::String(_) | ValueVariants::Integer(_), }) @@ -308,6 +307,19 @@ pub(super) fn condition_checker<'a, N: BoolIndexRead>( } } +struct BoolConditionChecker<'a, N> { + idx: &'a N, + is_true: bool, +} + +impl ConditionChecker for BoolConditionChecker<'_, N> { + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + Ok(self.idx.check_values_any(point_id, self.is_true)) + } +} + /// Produce a closure that maps a point id to its indexed bool values as JSON /// `Value`s. Shared by `BoolIndex::value_retriever` and /// `ReadOnlyBoolIndex::value_retriever`; both expose it via inherent methods diff --git a/lib/segment/src/index/field_index/full_text_index/read_ops.rs b/lib/segment/src/index/field_index/full_text_index/read_ops.rs index a0efeeebfb..5074b977c6 100644 --- a/lib/segment/src/index/field_index/full_text_index/read_ops.rs +++ b/lib/segment/src/index/field_index/full_text_index/read_ops.rs @@ -1,3 +1,4 @@ +use common::condition_checker::{ConditionChecker, ConstantConditionChecker}; use common::counter::hardware_accumulator::HwMeasurementAcc; use common::counter::hardware_counter::HardwareCounterCell; use common::types::PointOffsetType; @@ -8,7 +9,7 @@ use super::FullTextIndex; use super::full_text_index_read::{FullTextIndexRead, PayloadMatchQueryType}; use super::inverted_index::{ParsedQuery, TokenId}; use super::tokenizers::Tokenizer; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::index::field_index::{ CardinalityEstimation, PayloadBlockCondition, PayloadFieldIndexRead, }; @@ -319,14 +320,28 @@ pub fn condition_checker<'a, T: FullTextIndexRead>( }?; let Some(parsed_query) = query_opt else { - return Ok(Some(Box::new(|_| Ok(false)))); + return Ok(Some(Box::new(ConstantConditionChecker::MATCH_NONE))); }; - Ok(Some(Box::new(move |point_id| { - index.check_match(&parsed_query, point_id) + Ok(Some(Box::new(FullTextConditionChecker { + index, + parsed_query, }))) } +struct FullTextConditionChecker<'a, T> { + index: &'a T, + parsed_query: ParsedQuery, +} + +impl ConditionChecker for FullTextConditionChecker<'_, T> { + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + self.index.check_match(&self.parsed_query, point_id) + } +} + /// Body for [`PayloadFieldIndexRead::special_check_condition`]. Shared. pub fn special_check_condition( index: &T, diff --git a/lib/segment/src/index/field_index/geo_hash.rs b/lib/segment/src/index/field_index/geo_hash.rs index bc0a57770d..8eeaef9a6e 100644 --- a/lib/segment/src/index/field_index/geo_hash.rs +++ b/lib/segment/src/index/field_index/geo_hash.rs @@ -625,6 +625,7 @@ mod tests { use rand::{RngExt, SeedableRng}; use super::*; + use crate::types::CheckGeoPoint; use crate::types::test_utils::{build_polygon, build_polygon_with_interiors}; const BERLIN: GeoPoint = GeoPoint { diff --git a/lib/segment/src/index/field_index/geo_index/read_ops.rs b/lib/segment/src/index/field_index/geo_index/read_ops.rs index 6124f74540..6c0137514f 100644 --- a/lib/segment/src/index/field_index/geo_index/read_ops.rs +++ b/lib/segment/src/index/field_index/geo_index/read_ops.rs @@ -18,12 +18,13 @@ use std::cmp::{max, min}; use std::path::PathBuf; +use common::condition_checker::ConditionChecker; use common::counter::hardware_accumulator::HwMeasurementAcc; use common::counter::hardware_counter::HardwareCounterCell; use common::types::PointOffsetType; use super::GEO_QUERY_MAX_REGION; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::index::field_index::geo_hash::{ GeoHash, circle_hashes, common_hash_prefix, geo_hash_to_box, polygon_hashes, polygon_hashes_estimation, rectangle_hashes, @@ -33,7 +34,7 @@ use crate::index::field_index::{CardinalityEstimation, PayloadBlockCondition, Pr use crate::index::payload_config::StorageType; use crate::index::query_optimization::optimized_filter::DynConditionChecker; use crate::telemetry::PayloadIndexTelemetry; -use crate::types::{FieldCondition, GeoPoint, PayloadKeyType}; +use crate::types::{CheckGeoPoint, FieldCondition, GeoPoint, PayloadKeyType}; /// Shared read-only surface over the geo index variants. /// @@ -346,26 +347,46 @@ pub(super) fn condition_checker<'a, G: GeoIndexRead + ?Sized>( let hw_counter = hw_acc.get_counter_cell(); if let Some(geo_radius) = *geo_radius { - return Some(Box::new(move |point_id: PointOffsetType| { - geo.check_values_any(point_id, &hw_counter, &|value| { - geo_radius.check_point(value) - }) - })); + return Some(make_checker(geo, hw_counter, geo_radius)); } if let Some(geo_bounding_box) = *geo_bounding_box { - return Some(Box::new(move |point_id: PointOffsetType| { - geo.check_values_any(point_id, &hw_counter, &|value| { - geo_bounding_box.check_point(value) - }) - })); + return Some(make_checker(geo, hw_counter, geo_bounding_box)); } if let Some(geo_polygon) = geo_polygon.as_ref() { - let polygon_wrapper = geo_polygon.convert(); - return Some(Box::new(move |point_id: PointOffsetType| { - geo.check_values_any(point_id, &hw_counter, &|value| { - polygon_wrapper.check_point(value) - }) - })); + return Some(make_checker(geo, hw_counter, geo_polygon.convert())); } None } + +struct GeoConditionChecker<'a, G: ?Sized, F> { + geo: &'a G, + hw_counter: HardwareCounterCell, + filter: F, +} + +fn make_checker<'a>( + geo: &'a (impl GeoIndexRead + ?Sized), + hw_counter: HardwareCounterCell, + filter: impl CheckGeoPoint + 'a, +) -> DynConditionChecker<'a> { + Box::new(GeoConditionChecker { + geo, + hw_counter, + filter, + }) +} + +impl ConditionChecker for GeoConditionChecker<'_, G, P> +where + G: GeoIndexRead + ?Sized, + P: CheckGeoPoint, +{ + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + self.geo + .check_values_any(point_id, &self.hw_counter, &|value| { + self.filter.check_point(value) + }) + } +} diff --git a/lib/segment/src/index/field_index/geo_index/tests.rs b/lib/segment/src/index/field_index/geo_index/tests.rs index ab4a42d584..8233f672d1 100644 --- a/lib/segment/src/index/field_index/geo_index/tests.rs +++ b/lib/segment/src/index/field_index/geo_index/tests.rs @@ -27,7 +27,7 @@ use crate::index::field_index::{ use crate::json_path::JsonPath; use crate::types::test_utils::build_polygon; use crate::types::{ - FieldCondition, GeoBoundingBox, GeoLineString, GeoPoint, GeoPolygon, GeoRadius, + CheckGeoPoint, FieldCondition, GeoBoundingBox, GeoLineString, GeoPoint, GeoPolygon, GeoRadius, }; /// Generous default size for the deleted-points bitslice used in tests. diff --git a/lib/segment/src/index/field_index/map_index/immutable_map_index/read_ops.rs b/lib/segment/src/index/field_index/map_index/immutable_map_index/read_ops.rs index 359727f660..d922e1e2a4 100644 --- a/lib/segment/src/index/field_index/map_index/immutable_map_index/read_ops.rs +++ b/lib/segment/src/index/field_index/map_index/immutable_map_index/read_ops.rs @@ -12,10 +12,10 @@ use super::{ContainerSegment, ImmutableMapIndex}; use crate::common::operation_error::OperationResult; use crate::index::payload_config::StorageType; -impl MapIndexRead for ImmutableMapIndex +impl<'a, N, S> MapIndexRead<'a, N> for ImmutableMapIndex where Vec<::Owned>: Blob + Send + Sync, - N: MapIndexKey + ?Sized, + N: MapIndexKey + ?Sized + 'a, S: UniversalRead, { fn check_values_any( @@ -29,14 +29,11 @@ where .check_values_any(idx, |v| check_fn(v.borrow()))) } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, _hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { Some( self.point_to_values .get_values(idx)? diff --git a/lib/segment/src/index/field_index/map_index/mutable_map_index/in_memory.rs b/lib/segment/src/index/field_index/map_index/mutable_map_index/in_memory.rs index c1e9695136..6680ef3c51 100644 --- a/lib/segment/src/index/field_index/map_index/mutable_map_index/in_memory.rs +++ b/lib/segment/src/index/field_index/map_index/mutable_map_index/in_memory.rs @@ -103,7 +103,7 @@ where } } -impl MapIndexRead for InMemoryMapIndex +impl<'a, N: MapIndexKey + ?Sized + 'a> MapIndexRead<'a, N> for InMemoryMapIndex where Vec<::Owned>: Blob + Send + Sync, { @@ -120,14 +120,11 @@ where .unwrap_or(false)) } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, _hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { Some( self.point_to_values .get(idx as usize)? diff --git a/lib/segment/src/index/field_index/map_index/mutable_map_index/read_only/read_ops.rs b/lib/segment/src/index/field_index/map_index/mutable_map_index/read_only/read_ops.rs index ef09faee44..c22cb8cf3d 100644 --- a/lib/segment/src/index/field_index/map_index/mutable_map_index/read_only/read_ops.rs +++ b/lib/segment/src/index/field_index/map_index/mutable_map_index/read_only/read_ops.rs @@ -11,7 +11,8 @@ use super::ReadOnlyAppendableMapIndex; use crate::common::operation_error::OperationResult; use crate::index::payload_config::StorageType; -impl MapIndexRead for ReadOnlyAppendableMapIndex +impl<'a, N: MapIndexKey + ?Sized + 'a, S: UniversalRead> MapIndexRead<'a, N> + for ReadOnlyAppendableMapIndex where Vec<::Owned>: Blob + Send + Sync, { @@ -25,14 +26,11 @@ where .check_values_any(idx, hw_counter, check_fn) } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { self.in_memory_index.get_values(idx, hw_counter) } diff --git a/lib/segment/src/index/field_index/map_index/mutable_map_index/read_ops.rs b/lib/segment/src/index/field_index/map_index/mutable_map_index/read_ops.rs index a36cc4eccc..2287619f2f 100644 --- a/lib/segment/src/index/field_index/map_index/mutable_map_index/read_ops.rs +++ b/lib/segment/src/index/field_index/map_index/mutable_map_index/read_ops.rs @@ -10,7 +10,7 @@ use super::MutableMapIndex; use crate::common::operation_error::OperationResult; use crate::index::payload_config::StorageType; -impl MapIndexRead for MutableMapIndex +impl<'a, N: MapIndexKey + ?Sized + 'a> MapIndexRead<'a, N> for MutableMapIndex where Vec<::Owned>: Blob + Send + Sync, { @@ -24,14 +24,11 @@ where .check_values_any(idx, hw_counter, check_fn) } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { self.in_memory_index.get_values(idx, hw_counter) } diff --git a/lib/segment/src/index/field_index/map_index/on_disk_map_index/read_ops.rs b/lib/segment/src/index/field_index/map_index/on_disk_map_index/read_ops.rs index bcde5c1b1b..cb55194bb7 100644 --- a/lib/segment/src/index/field_index/map_index/on_disk_map_index/read_ops.rs +++ b/lib/segment/src/index/field_index/map_index/on_disk_map_index/read_ops.rs @@ -17,7 +17,9 @@ use crate::common::operation_error::OperationResult; use crate::index::field_index::on_disk_point_to_values::ValuesIter; use crate::index::payload_config::StorageType; -impl MapIndexRead for OnDiskMapIndex { +impl<'a, N: MapIndexKey + Key + ?Sized + 'a, S: UniversalRead> MapIndexRead<'a, N> + for OnDiskMapIndex +{ fn check_values_any( &self, idx: PointOffsetType, @@ -46,14 +48,11 @@ impl MapIndexRead for OnDisk .check_values_any(idx, |v| check_fn(v), &hw_counter) } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { let hw_counter = ConditionedCounter::always(hw_counter); // We can account cost of reading `bool`, but it will likely be more expensive, than diff --git a/lib/segment/src/index/field_index/map_index/payload_index_impl/int.rs b/lib/segment/src/index/field_index/map_index/payload_index_impl/int.rs index 1f8c9244bb..a624c1ac5c 100644 --- a/lib/segment/src/index/field_index/map_index/payload_index_impl/int.rs +++ b/lib/segment/src/index/field_index/map_index/payload_index_impl/int.rs @@ -19,7 +19,6 @@ use crate::index::field_index::{ }; use crate::index::query_estimator::combine_should_estimations; use crate::index::query_optimization::optimized_filter::DynConditionChecker; -use crate::payload_storage::condition_checker::INDEXSET_ITER_THRESHOLD; use crate::types::{ AnyVariants, FieldCondition, IntPayloadType, Match, MatchAny, MatchExcept, MatchValue, PayloadKeyType, ValueVariants, @@ -127,7 +126,7 @@ where // Shared bodies for `MapIndex` and // `ReadOnlyMapIndex`. -fn filter_impl<'a, T: MapIndexRead>( +fn filter_impl<'a, T: MapIndexRead<'a, IntPayloadType>>( index: &'a T, condition: &'a FieldCondition, hw_counter: &'a HardwareCounterCell, @@ -165,8 +164,8 @@ fn filter_impl<'a, T: MapIndexRead>( Ok(result) } -fn estimate_cardinality_impl>( - index: &T, +fn estimate_cardinality_impl<'a, T: MapIndexRead<'a, IntPayloadType>>( + index: &'a T, condition: &FieldCondition, hw_counter: &HardwareCounterCell, ) -> Option { @@ -219,8 +218,8 @@ fn estimate_cardinality_impl>( } } -fn for_each_payload_block_impl>( - index: &T, +fn for_each_payload_block_impl<'a, T: MapIndexRead<'a, IntPayloadType>>( + index: &'a T, threshold: usize, key: PayloadKeyType, f: &mut dyn FnMut(PayloadBlockCondition) -> OperationResult<()>, @@ -240,7 +239,7 @@ fn for_each_payload_block_impl>( }) } -fn condition_checker_impl<'a, T: MapIndexRead + 'a>( +fn condition_checker_impl<'a, T: MapIndexRead<'a, IntPayloadType> + 'a>( index: &'a T, condition: &FieldCondition, hw_acc: HwMeasurementAcc, @@ -264,44 +263,13 @@ fn condition_checker_impl<'a, T: MapIndexRead + 'a>( match cond_match { Match::Value(MatchValue { value: ValueVariants::Integer(value), - }) => { - let value = *value; - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |i| *i == value) - })) - } + }) => Some(index.match_value_checker(hw_counter, *value)), Match::Any(MatchAny { any: AnyVariants::Integers(list), - }) => { - let list = list.clone(); - if list.len() < INDEXSET_ITER_THRESHOLD { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| { - list.iter().any(|i| i == value) - }) - })) - } else { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| list.contains(value)) - })) - } - } + }) => Some(index.match_any_checker(hw_counter, list.clone(), false)), Match::Except(MatchExcept { except: AnyVariants::Integers(list), - }) => { - let list = list.clone(); - if list.len() < INDEXSET_ITER_THRESHOLD { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| { - !list.iter().any(|i| i == value) - }) - })) - } else { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| !list.contains(value)) - })) - } - } + }) => Some(index.match_any_checker(hw_counter, list.clone(), true)), // Conditions this index can't serve. Match::Value(MatchValue { value: ValueVariants::String(_) | ValueVariants::Bool(_), diff --git a/lib/segment/src/index/field_index/map_index/payload_index_impl/str.rs b/lib/segment/src/index/field_index/map_index/payload_index_impl/str.rs index 04c6b3bd1c..4bbcfa45ba 100644 --- a/lib/segment/src/index/field_index/map_index/payload_index_impl/str.rs +++ b/lib/segment/src/index/field_index/map_index/payload_index_impl/str.rs @@ -20,7 +20,6 @@ use crate::index::field_index::{ }; use crate::index::query_estimator::combine_should_estimations; use crate::index::query_optimization::optimized_filter::DynConditionChecker; -use crate::payload_storage::condition_checker::INDEXSET_ITER_THRESHOLD; use crate::types::{ AnyVariants, FieldCondition, Match, MatchAny, MatchExcept, MatchValue, PayloadKeyType, ValueVariants, @@ -129,7 +128,7 @@ where // over `T: MapIndexRead` so a single body serves both `PayloadFieldIndexRead` // impls above. -fn filter_impl<'a, T: MapIndexRead>( +fn filter_impl<'a, T: MapIndexRead<'a, str>>( index: &'a T, condition: &'a FieldCondition, hw_counter: &'a HardwareCounterCell, @@ -167,8 +166,8 @@ fn filter_impl<'a, T: MapIndexRead>( Ok(result) } -fn estimate_cardinality_impl>( - index: &T, +fn estimate_cardinality_impl<'a, T: MapIndexRead<'a, str>>( + index: &'a T, condition: &FieldCondition, hw_counter: &HardwareCounterCell, ) -> Option { @@ -221,8 +220,8 @@ fn estimate_cardinality_impl>( } } -fn for_each_payload_block_impl>( - index: &T, +fn for_each_payload_block_impl<'a, T: MapIndexRead<'a, str>>( + index: &'a T, threshold: usize, key: PayloadKeyType, f: &mut dyn FnMut(PayloadBlockCondition) -> OperationResult<()>, @@ -243,7 +242,7 @@ fn for_each_payload_block_impl>( }) } -fn condition_checker_impl<'a, T: MapIndexRead + 'a>( +fn condition_checker_impl<'a, T: MapIndexRead<'a, str> + 'a>( index: &'a T, condition: &FieldCondition, hw_acc: HwMeasurementAcc, @@ -267,44 +266,13 @@ fn condition_checker_impl<'a, T: MapIndexRead + 'a>( match cond_match { Match::Value(MatchValue { value: ValueVariants::String(keyword), - }) => { - let keyword = keyword.clone(); - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| value == keyword.as_str()) - })) - } + }) => Some(index.match_value_checker(hw_counter, keyword.clone())), Match::Any(MatchAny { any: AnyVariants::Strings(list), - }) => { - let list = list.clone(); - if list.len() < INDEXSET_ITER_THRESHOLD { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| { - list.iter().any(|s| s.as_str() == value) - }) - })) - } else { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| list.contains(value)) - })) - } - } + }) => Some(index.match_any_checker(hw_counter, list.clone(), false)), Match::Except(MatchExcept { except: AnyVariants::Strings(list), - }) => { - let list = list.clone(); - if list.len() < INDEXSET_ITER_THRESHOLD { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| { - !list.iter().any(|s| s.as_str() == value) - }) - })) - } else { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| !list.contains(value)) - })) - } - } + }) => Some(index.match_any_checker(hw_counter, list.clone(), true)), // Conditions this index can't serve: Match::Text/TextAny/Phrase // (handled by FullTextIndex) and value-type mismatches (e.g. // Match::Value(Integer) against a string-keyed map). diff --git a/lib/segment/src/index/field_index/map_index/payload_index_impl/uuid.rs b/lib/segment/src/index/field_index/map_index/payload_index_impl/uuid.rs index 4460a7c3d8..54acc2949b 100644 --- a/lib/segment/src/index/field_index/map_index/payload_index_impl/uuid.rs +++ b/lib/segment/src/index/field_index/map_index/payload_index_impl/uuid.rs @@ -23,7 +23,6 @@ use crate::index::field_index::{ }; use crate::index::query_estimator::combine_should_estimations; use crate::index::query_optimization::optimized_filter::DynConditionChecker; -use crate::payload_storage::condition_checker::INDEXSET_ITER_THRESHOLD; use crate::types::{ AnyVariants, FieldCondition, Match, MatchAny, MatchExcept, MatchValue, PayloadKeyType, UuidIntType, ValueVariants, @@ -131,7 +130,7 @@ where // Shared bodies for `MapIndex` and // `ReadOnlyMapIndex`. -fn filter_impl<'a, T: MapIndexRead>( +fn filter_impl<'a, T: MapIndexRead<'a, UuidIntType>>( index: &'a T, condition: &'a FieldCondition, hw_counter: &'a HardwareCounterCell, @@ -200,8 +199,8 @@ fn filter_impl<'a, T: MapIndexRead>( Ok(result) } -fn estimate_cardinality_impl>( - index: &T, +fn estimate_cardinality_impl<'a, T: MapIndexRead<'a, UuidIntType>>( + index: &'a T, condition: &FieldCondition, hw_counter: &HardwareCounterCell, ) -> OperationResult> { @@ -275,8 +274,8 @@ fn estimate_cardinality_impl>( }) } -fn for_each_payload_block_impl>( - index: &T, +fn for_each_payload_block_impl<'a, T: MapIndexRead<'a, UuidIntType>>( + index: &'a T, threshold: usize, key: PayloadKeyType, f: &mut dyn FnMut(PayloadBlockCondition) -> OperationResult<()>, @@ -299,7 +298,7 @@ fn for_each_payload_block_impl>( }) } -fn condition_checker_impl<'a, T: MapIndexRead + 'a>( +fn condition_checker_impl<'a, T: MapIndexRead<'a, UuidIntType> + 'a>( index: &'a T, condition: &FieldCondition, hw_acc: HwMeasurementAcc, @@ -325,9 +324,7 @@ fn condition_checker_impl<'a, T: MapIndexRead + 'a>( value: ValueVariants::String(keyword), }) => { let uuid = Uuid::parse_str(keyword).map(|u| u.as_u128()).ok()?; - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| value == &uuid) - })) + Some(index.match_value_checker(hw_counter, uuid)) } Match::Any(MatchAny { any: AnyVariants::Strings(list), @@ -336,17 +333,7 @@ fn condition_checker_impl<'a, T: MapIndexRead + 'a>( .iter() .map(|s| Uuid::parse_str(s).map(|u| u.as_u128()).ok()) .collect::>>()?; - if list.len() < INDEXSET_ITER_THRESHOLD { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| { - list.iter().any(|i| i == value) - }) - })) - } else { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| list.contains(value)) - })) - } + Some(index.match_any_checker(hw_counter, list, false)) } Match::Except(MatchExcept { except: AnyVariants::Strings(list), @@ -355,17 +342,7 @@ fn condition_checker_impl<'a, T: MapIndexRead + 'a>( .iter() .map(|s| Uuid::parse_str(s).map(|u| u.as_u128()).ok()) .collect::>>()?; - if list.len() < INDEXSET_ITER_THRESHOLD { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| { - !list.iter().any(|i| i == value) - }) - })) - } else { - Some(Box::new(move |point_id: PointOffsetType| { - index.check_values_any(point_id, &hw_counter, |value| !list.contains(value)) - })) - } + Some(index.match_any_checker(hw_counter, list, true)) } // Conditions this index can't serve. Match::Value(MatchValue { diff --git a/lib/segment/src/index/field_index/map_index/read_only/read_ops.rs b/lib/segment/src/index/field_index/map_index/read_only/read_ops.rs index 8110956dd5..34e7775873 100644 --- a/lib/segment/src/index/field_index/map_index/read_only/read_ops.rs +++ b/lib/segment/src/index/field_index/map_index/read_only/read_ops.rs @@ -17,7 +17,8 @@ use crate::index::payload_config::StorageType; /// `except_set`, `values_is_empty`) are picked up from the trait's default /// impls — they only depend on the required methods, so no per-variant /// dispatch is needed for them. -impl MapIndexRead for ReadOnlyMapIndex +impl<'a, N: MapIndexKey + Key + ?Sized + 'a, S: UniversalRead> MapIndexRead<'a, N> + for ReadOnlyMapIndex where Vec<::Owned>: Blob + Send + Sync, { @@ -36,14 +37,11 @@ where } } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { let boxed: Box> + 'a> = match self { ReadOnlyMapIndex::Appendable(index) => Box::new(index.get_values(idx, hw_counter)?), ReadOnlyMapIndex::Immutable(index) => Box::new(index.get_values(idx, hw_counter)?), diff --git a/lib/segment/src/index/field_index/map_index/read_ops.rs b/lib/segment/src/index/field_index/map_index/read_ops.rs index f204e4a8e6..8169963c7f 100644 --- a/lib/segment/src/index/field_index/map_index/read_ops.rs +++ b/lib/segment/src/index/field_index/map_index/read_ops.rs @@ -1,6 +1,8 @@ use std::borrow::{Borrow, Cow}; use std::hash::{BuildHasher, Hash}; +use std::marker::PhantomData; +use common::condition_checker::ConditionChecker; use common::counter::hardware_counter::HardwareCounterCell; use common::types::PointOffsetType; use gridstore::Blob; @@ -8,10 +10,12 @@ use indexmap::IndexSet; use super::key::MapIndexKey; use super::{IdIter, MapIndex}; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::index::field_index::CardinalityEstimation; use crate::index::field_index::stat_tools::number_of_selected_points; use crate::index::payload_config::{IndexMutability, StorageType}; +use crate::index::query_optimization::optimized_filter::DynConditionChecker; +use crate::payload_storage::condition_checker::INDEXSET_ITER_THRESHOLD; use crate::telemetry::PayloadIndexTelemetry; /// Read-only operations supported by every map-index storage variant @@ -23,7 +27,7 @@ use crate::telemetry::PayloadIndexTelemetry; /// [`MapIndex`] can call them generically. Variants that don't need /// `hw_counter` (`Mutable` / `Immutable`) accept and ignore it; the /// storage-backed `Universal` variant uses it to track payload-index IO. -pub trait MapIndexRead { +pub trait MapIndexRead<'a, N: MapIndexKey + ?Sized + 'a>: Sized { fn check_values_any( &self, idx: PointOffsetType, @@ -31,13 +35,11 @@ pub trait MapIndexRead { check_fn: impl Fn(&N) -> bool, ) -> OperationResult; - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a; + ) -> Option> + 'a>; fn values_count(&self, idx: PointOffsetType) -> Option; @@ -120,13 +122,13 @@ pub trait MapIndexRead { /// # Returns /// /// * `CardinalityEstimation` - estimation of cardinality - fn except_cardinality<'a>( - &'a self, - excluded: impl Iterator, + fn except_cardinality<'b>( + &self, + excluded: impl Iterator, hw_counter: &HardwareCounterCell, ) -> CardinalityEstimation where - N: 'a, + N: 'b, { // Minimal case: we exclude as many points as possible. let excluded_value_counts: Vec<_> = excluded @@ -174,7 +176,7 @@ pub trait MapIndexRead { } } - fn except_set<'a, K, A>( + fn except_set( &'a self, excluded: &'a IndexSet, hw_counter: &'a HardwareCounterCell, @@ -194,9 +196,53 @@ pub trait MapIndexRead { })?; Ok(Box::new(points.into_iter())) } + + /// Condition checker for [`crate::types::Match::Value`]. + fn match_value_checker( + &'a self, + hw_counter: HardwareCounterCell, + value: impl Borrow + 'a, + ) -> DynConditionChecker<'a> { + Box::new(MapConditionChecker { + index: self, + hw_counter, + predicate: move |v: &N| v == value.borrow(), + _key: PhantomData, + }) + } + + /// Condition checker for + /// - [`crate::types::Match::Any`] (when `negate` is `false`), + /// - [`crate::types::Match::Except`] (when `negate` is `true`). + fn match_any_checker( + &'a self, + hw_counter: HardwareCounterCell, + list: IndexSet, + negate: bool, + ) -> DynConditionChecker<'a> + where + A: BuildHasher + 'a, + K: Borrow + Hash + Eq + 'a, + { + if list.len() < INDEXSET_ITER_THRESHOLD { + Box::new(MapConditionChecker { + index: self, + hw_counter, + predicate: move |value: &N| list.iter().any(|e| e.borrow() == value) != negate, + _key: PhantomData, + }) + } else { + Box::new(MapConditionChecker { + index: self, + hw_counter, + predicate: move |value: &N| list.contains(value) != negate, + _key: PhantomData, + }) + } + } } -impl MapIndexRead for MapIndex +impl<'a, N: MapIndexKey + ?Sized + 'a> MapIndexRead<'a, N> for MapIndex where Vec<::Owned>: Blob + Send + Sync, { @@ -213,14 +259,11 @@ where } } - fn get_values<'a>( + fn get_values( &'a self, idx: PointOffsetType, hw_counter: &HardwareCounterCell, - ) -> Option> + 'a> - where - N: 'a, - { + ) -> Option> + 'a> { let boxed: Box> + 'a> = match self { MapIndex::Mutable(index) => Box::new(index.get_values(idx, hw_counter)?), MapIndex::Immutable(index) => Box::new(index.get_values(idx, hw_counter)?), @@ -347,8 +390,8 @@ where pub fn get_telemetry_data(&self) -> PayloadIndexTelemetry { PayloadIndexTelemetry { field_name: None, - points_count: >::get_indexed_points(self), - points_values_count: >::get_values_count(self), + points_count: >::get_indexed_points(self), + points_values_count: >::get_values_count(self), histogram_bucket_size: None, index_type: match self { MapIndex::Mutable(_) => "mutable_map", @@ -362,13 +405,13 @@ where /// [`MapIndexRead::values_count`] for callers outside this module who /// don't have the (`pub(super)`) trait in scope. pub fn values_count(&self, idx: PointOffsetType) -> usize { - >::values_count(self, idx).unwrap_or(0) + >::values_count(self, idx).unwrap_or(0) } /// `bool`-returning convenience wrapper around /// [`MapIndexRead::values_is_empty`]; see [`Self::values_count`] for why. pub fn values_is_empty(&self, idx: PointOffsetType) -> bool { - >::values_is_empty(self, idx) + >::values_is_empty(self, idx) } /// Convenience wrapper around [`MapIndexRead::get_values`] that boxes the @@ -379,14 +422,14 @@ where idx: PointOffsetType, hw_counter: &HardwareCounterCell, ) -> Option> + '_>> { - let iter = >::get_values(self, idx, hw_counter)?; + let iter = >::get_values(self, idx, hw_counter)?; Some(Box::new(iter)) } /// Convenience wrapper around [`MapIndexRead::ram_usage_bytes`]; see /// [`Self::values_count`] for why. pub fn ram_usage_bytes(&self) -> usize { - >::ram_usage_bytes(self) + >::ram_usage_bytes(self) } pub fn is_on_disk(&self) -> bool { @@ -413,3 +456,24 @@ where } } } + +struct MapConditionChecker<'a, T, N: ?Sized, F> { + index: &'a T, + hw_counter: HardwareCounterCell, + predicate: F, + _key: PhantomData, +} + +impl<'a, N, T, F> ConditionChecker for MapConditionChecker<'a, T, N, F> +where + N: MapIndexKey + ?Sized + 'a, + T: MapIndexRead<'a, N>, + F: Fn(&N) -> bool, +{ + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + self.index + .check_values_any(point_id, &self.hw_counter, &self.predicate) + } +} diff --git a/lib/segment/src/index/field_index/map_index/value_indexer_impl.rs b/lib/segment/src/index/field_index/map_index/value_indexer_impl.rs index 623a3d898d..0a9c609562 100644 --- a/lib/segment/src/index/field_index/map_index/value_indexer_impl.rs +++ b/lib/segment/src/index/field_index/map_index/value_indexer_impl.rs @@ -179,7 +179,7 @@ where // Shared per-K bodies, parameterized over `T: MapIndexRead` so a single // implementation serves both `MapIndex` and `ReadOnlyMapIndex`. -fn value_retriever_str<'a, T: MapIndexRead + 'a>( +fn value_retriever_str<'a, T: MapIndexRead<'a, str> + 'a>( index: &'a T, hw_counter: &'a HardwareCounterCell, ) -> VariableRetrieverFn<'a> { @@ -193,7 +193,7 @@ fn value_retriever_str<'a, T: MapIndexRead + 'a>( }) } -fn value_retriever_int<'a, T: MapIndexRead + 'a>( +fn value_retriever_int<'a, T: MapIndexRead<'a, IntPayloadType> + 'a>( index: &'a T, hw_counter: &'a HardwareCounterCell, ) -> VariableRetrieverFn<'a> { @@ -207,7 +207,7 @@ fn value_retriever_int<'a, T: MapIndexRead + 'a>( }) } -fn value_retriever_uuid<'a, T: MapIndexRead + 'a>( +fn value_retriever_uuid<'a, T: MapIndexRead<'a, UuidIntType> + 'a>( index: &'a T, hw_counter: &'a HardwareCounterCell, ) -> VariableRetrieverFn<'a> { diff --git a/lib/segment/src/index/field_index/null_index/read_ops.rs b/lib/segment/src/index/field_index/null_index/read_ops.rs index 89188e6457..f5291ee2ce 100644 --- a/lib/segment/src/index/field_index/null_index/read_ops.rs +++ b/lib/segment/src/index/field_index/null_index/read_ops.rs @@ -18,11 +18,12 @@ use std::path::PathBuf; +use common::condition_checker::ConditionChecker; use common::counter::hardware_accumulator::HwMeasurementAcc; use common::types::PointOffsetType; use crate::common::flags::roaring_flags::RoaringFlagsRead; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::index::field_index::{CardinalityEstimation, PrimaryCondition}; use crate::index::payload_config::StorageType; use crate::index::query_optimization::optimized_filter::DynConditionChecker; @@ -241,14 +242,42 @@ pub(super) fn condition_checker<'a, N: NullIndexRead>( } = condition; if let Some(is_empty) = *is_empty { - return Some(Box::new(move |point_id: PointOffsetType| { - Ok(null_index.values_is_empty(point_id) == is_empty) + return Some(Box::new(IsEmptyConditionChecker { + null_index, + is_empty, })); } if let Some(is_null) = *is_null { - return Some(Box::new(move |point_id: PointOffsetType| { - Ok(null_index.values_is_null(point_id) == is_null) + return Some(Box::new(IsNullConditionChecker { + null_index, + is_null, })); } None } + +struct IsEmptyConditionChecker<'a, N> { + null_index: &'a N, + is_empty: bool, +} + +impl ConditionChecker for IsEmptyConditionChecker<'_, N> { + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + Ok(self.null_index.values_is_empty(point_id) == self.is_empty) + } +} + +struct IsNullConditionChecker<'a, N> { + null_index: &'a N, + is_null: bool, +} + +impl ConditionChecker for IsNullConditionChecker<'_, N> { + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + Ok(self.null_index.values_is_null(point_id) == self.is_null) + } +} diff --git a/lib/segment/src/index/field_index/numeric_index/query.rs b/lib/segment/src/index/field_index/numeric_index/query.rs index 1c7ebd0686..1a84ef127d 100644 --- a/lib/segment/src/index/field_index/numeric_index/query.rs +++ b/lib/segment/src/index/field_index/numeric_index/query.rs @@ -12,6 +12,7 @@ use std::ops::Bound; use std::ops::Bound::{Excluded, Included, Unbounded}; use std::str::FromStr; +use common::condition_checker::ConditionChecker; use common::counter::hardware_accumulator::HwMeasurementAcc; use common::counter::hardware_counter::HardwareCounterCell; use common::types::PointOffsetType; @@ -21,7 +22,7 @@ use uuid::Uuid; use super::Encodable; use super::numeric_index_read::NumericIndexRead; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::index::field_index::numeric_point::{Numericable, Point}; use crate::index::field_index::on_disk_point_to_values::StoredValue; use crate::index::field_index::stat_tools::estimate_multi_value_selection_cardinality; @@ -298,8 +299,8 @@ where collect_blocks()?.into_iter().try_for_each(f) } -/// Build a per-point checker closure for a `range` field condition, if the -/// index can serve it. +/// Build a per-point checker for a `range` field condition, if the index can +/// serve it. pub(super) fn condition_checker<'a, T, I>( index: &'a I, condition: &FieldCondition, @@ -337,16 +338,35 @@ where } }; - let hw_counter = hw_acc.get_counter_cell(); - Some(Box::new(move |point_id: PointOffsetType| { - Ok(index.check_values_any( - point_id, - |value| typed_range.check_range(*value), - &hw_counter, - )) + Some(Box::new(RangeConditionChecker { + index, + typed_range, + hw_counter: hw_acc.get_counter_cell(), })) } +struct RangeConditionChecker<'a, T, I> { + index: &'a I, + typed_range: Range, + hw_counter: HardwareCounterCell, +} + +impl ConditionChecker for RangeConditionChecker<'_, T, I> +where + T: Encodable + Numericable + StoredValue + Send + Sync + Default, + I: NumericIndexRead, +{ + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + Ok(self.index.check_values_any( + point_id, + |value| self.typed_range.check_range(*value), + &self.hw_counter, + )) + } +} + /// Stream `(value, point)` pairs of the given range in ascending order. /// /// The iterator is double-ended, so callers can also walk it in diff --git a/lib/segment/src/index/query_optimization/rescore_formula/formula_scorer.rs b/lib/segment/src/index/query_optimization/rescore_formula/formula_scorer.rs index 25c71f3e55..8036ef3d12 100644 --- a/lib/segment/src/index/query_optimization/rescore_formula/formula_scorer.rs +++ b/lib/segment/src/index/query_optimization/rescore_formula/formula_scorer.rs @@ -347,6 +347,7 @@ fn linear_decay(x: PreciseScore, target: PreciseScore, lambda: PreciseScore) -> mod tests { use std::collections::HashMap; + use common::condition_checker::ConstantConditionChecker; use rstest::rstest; use serde_json::json; use smallvec::smallvec; @@ -400,8 +401,8 @@ mod tests { ); let condition_checkers = vec![ - OptimizedCondition::Checker(Box::new(|_| Ok(true))), - OptimizedCondition::Checker(Box::new(|_| Ok(false))), + OptimizedCondition::Checker(Box::new(ConstantConditionChecker::MATCH_ALL)), + OptimizedCondition::Checker(Box::new(ConstantConditionChecker::MATCH_NONE)), ]; FormulaScorer { diff --git a/lib/segment/src/index/struct_payload_index/read_view/condition_converter.rs b/lib/segment/src/index/struct_payload_index/read_view/condition_converter.rs index 39d2bf418e..b8a8e0c736 100644 --- a/lib/segment/src/index/struct_payload_index/read_view/condition_converter.rs +++ b/lib/segment/src/index/struct_payload_index/read_view/condition_converter.rs @@ -1,12 +1,15 @@ use std::collections::HashMap; +use std::sync::Arc; use ahash::AHashSet; +use atomic_refcell::AtomicRefCell; +use common::condition_checker::{ConditionChecker, ConstantConditionChecker}; use common::counter::hardware_counter::HardwareCounterCell; -use common::types::DeferredBehavior; +use common::types::{DeferredBehavior, PointOffsetType}; use serde_json::Value; use super::StructPayloadIndexReadView; -use crate::common::operation_error::OperationResult; +use crate::common::operation_error::{OperationError, OperationResult}; use crate::id_tracker::IdTrackerRead; use crate::index::field_index::FieldIndexRead; use crate::index::query_optimization::optimized_filter::DynConditionChecker; @@ -85,17 +88,15 @@ where id_tracker.internal_id_with_behavior(*external_id, deferred_behavior) }) .collect(); - Box::new(move |point_id| Ok(segment_ids.contains(&point_id))) + Box::new(IdsConditionChecker(segment_ids)) } Condition::HasVector(has_vector) => { if let Some(vector_storage) = self.vector_storages.get(&has_vector.has_vector).cloned() { - Box::new(move |point_id| { - Ok(!vector_storage.borrow().is_deleted_vector(point_id)) - }) + Box::new(HasVectorConditionChecker(vector_storage)) } else { - Box::new(|_point_id| Ok(false)) + Box::new(ConstantConditionChecker::MATCH_NONE) } } Condition::Nested(nested) => { @@ -121,37 +122,34 @@ where let nested_indexes = select_nested_indexes(&nested_path, field_indexes); - let hw = hw_counter.fork(); - Box::new(move |point_id| { - payload_provider.with_payload( - point_id, - |payload| { - let field_values = payload.get_value(&nested_path); + Box::new(PayloadConditionChecker { + payload_provider, + hw_counter: hw_counter.fork(), + check: move |payload, point_id, hw| { + let field_values = payload.get_value(&nested_path); - for value in field_values { - if let Value::Object(object) = value { - let get_payload = || OwnedPayloadRef::from(object); - if check_payload( - Box::new(get_payload), - // None because has_id in nested is not supported. So retrieving - // IDs through the tracker would always return None. - None, - // Same as above, nested conditions don't support has_vector. - &HashMap::new(), - &nested.nested.filter, - point_id, - &nested_indexes, - &hw, - ) { - // If at least one nested object matches, return true - return Ok(true); - } + for value in field_values { + if let Value::Object(object) = value { + let get_payload = || OwnedPayloadRef::from(object); + if check_payload( + Box::new(get_payload), + // None because has_id in nested is not supported. So retrieving + // IDs through the tracker would always return None. + None, + // Same as above, nested conditions don't support has_vector. + &HashMap::new(), + &nested.nested.filter, + point_id, + &nested_indexes, + hw, + ) { + // If at least one nested object matches, return true + return Ok(true); } } - Ok(false) - }, - &hw, - ) + } + Ok(false) + }, }) } Condition::CustomIdChecker(cond) => { @@ -164,13 +162,14 @@ where }) .collect(); - Box::new(move |internal_id| Ok(segment_ids.contains(&internal_id))) + Box::new(IdsConditionChecker(segment_ids)) } Condition::Filter(_) => unreachable!(), }) } } +/// For [`Condition::Field`], [`Condition::IsEmpty`] and [`Condition::IsNull`]. fn field_condition_checker<'a>( field_indexes: &'a HashMap>, key: &JsonPath, @@ -190,8 +189,58 @@ fn field_condition_checker<'a>( } // 2. None found => fallback to payload check. - let hw_counter = hw_counter.fork(); - Ok(Box::new(move |point_id| { - payload_provider.with_payload(point_id, |payload| check(payload, &hw_counter), &hw_counter) + Ok(Box::new(PayloadConditionChecker { + payload_provider, + hw_counter: hw_counter.fork(), + check: move |payload, _, hw| check(payload, hw), })) } + +/// For [`field_condition_checker`] and [`Condition::Nested`]. +struct PayloadConditionChecker +where + S: PayloadStorageRead, + F: Fn(OwnedPayloadRef, PointOffsetType, &HardwareCounterCell) -> OperationResult, +{ + payload_provider: PayloadProvider, + hw_counter: HardwareCounterCell, + check: F, +} + +impl ConditionChecker for PayloadConditionChecker +where + S: PayloadStorageRead, + F: Fn(OwnedPayloadRef, PointOffsetType, &HardwareCounterCell) -> OperationResult, +{ + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + self.payload_provider.with_payload( + point_id, + |payload| (self.check)(payload, point_id, &self.hw_counter), + &self.hw_counter, + ) + } +} + +/// For [`Condition::HasId`] and [`Condition::CustomIdChecker`]. +struct IdsConditionChecker(AHashSet); + +impl ConditionChecker for IdsConditionChecker { + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + Ok(self.0.contains(&point_id)) + } +} + +/// For [`Condition::HasVector`]. +struct HasVectorConditionChecker(Arc>); + +impl ConditionChecker for HasVectorConditionChecker { + type Error = OperationError; + + fn check(&self, point_id: PointOffsetType) -> OperationResult { + Ok(!self.0.borrow().is_deleted_vector(point_id)) + } +} diff --git a/lib/segment/src/payload_storage/condition_checker.rs b/lib/segment/src/payload_storage/condition_checker.rs index cc889544ce..c1d7ccb7ad 100644 --- a/lib/segment/src/payload_storage/condition_checker.rs +++ b/lib/segment/src/payload_storage/condition_checker.rs @@ -6,9 +6,9 @@ use ordered_float::OrderedFloat; use serde_json::Value; use crate::types::{ - AnyVariants, DateTimePayloadType, FieldCondition, FloatPayloadType, GeoBoundingBox, GeoPoint, - GeoPolygon, GeoRadius, Match, MatchAny, MatchExcept, MatchPhrase, MatchText, MatchTextAny, - MatchValue, Range, RangeInterface, ValueVariants, ValuesCount, + AnyVariants, CheckGeoPoint, DateTimePayloadType, FieldCondition, FloatPayloadType, + GeoBoundingBox, GeoPoint, GeoPolygon, GeoRadius, Match, MatchAny, MatchExcept, MatchPhrase, + MatchText, MatchTextAny, MatchValue, Range, RangeInterface, ValueVariants, ValuesCount, }; /// Threshold representing the point to which iterating through an IndexSet is more efficient than using hashing. diff --git a/lib/segment/src/types.rs b/lib/segment/src/types.rs index d0bd6661a6..752078ba5c 100644 --- a/lib/segment/src/types.rs +++ b/lib/segment/src/types.rs @@ -2921,6 +2921,10 @@ impl From> for ValuesCount { } } +pub trait CheckGeoPoint { + fn check_point(&self, point: &GeoPoint) -> bool; +} + /// Geo filter request /// /// Matches coordinates inside the rectangle, described by coordinates of lop-left and bottom-right edges @@ -2933,8 +2937,8 @@ pub struct GeoBoundingBox { pub bottom_right: GeoPoint, } -impl GeoBoundingBox { - pub fn check_point(&self, point: &GeoPoint) -> bool { +impl CheckGeoPoint for GeoBoundingBox { + fn check_point(&self, point: &GeoPoint) -> bool { let longitude_check = if self.top_left.lon > self.bottom_right.lon { // Handle antimeridian crossing point.lon > self.top_left.lon || point.lon < self.bottom_right.lon @@ -2969,8 +2973,8 @@ impl Hash for GeoRadius { } } -impl GeoRadius { - pub fn check_point(&self, point: &GeoPoint) -> bool { +impl CheckGeoPoint for GeoRadius { + fn check_point(&self, point: &GeoPoint) -> bool { let query_center = Point::from(self.center); Haversine.distance(query_center, Point::from(*point)) < self.radius.0 } @@ -2986,8 +2990,8 @@ pub struct PolygonWrapper { pub polygon: Polygon, } -impl PolygonWrapper { - pub fn check_point(&self, point: &GeoPoint) -> bool { +impl CheckGeoPoint for PolygonWrapper { + fn check_point(&self, point: &GeoPoint) -> bool { let point_new = Point::new(point.lon.0, point.lat.0); self.polygon.contains(&point_new) }