diff --git a/lib/collection/src/operations/verification/query.rs b/lib/collection/src/operations/verification/query.rs index 7d3b663617..e250ddca9b 100644 --- a/lib/collection/src/operations/verification/query.rs +++ b/lib/collection/src/operations/verification/query.rs @@ -1,4 +1,5 @@ -use segment::types::StrictModeConfig; +use segment::data_types::vectors::DEFAULT_VECTOR_NAME; +use segment::types::{Filter, StrictModeConfig}; use super::{StrictModeVerification, check_grouping_field}; use crate::collection::Collection; @@ -40,6 +41,7 @@ impl Query { async fn check_fullscan( &self, using: &str, + filter: Option<&Filter>, collection: &Collection, strict_mode_config: &StrictModeConfig, ) -> CollectionResult<()> { @@ -68,16 +70,60 @@ impl Query { .get_params(using) .and_then(|param| param.hnsw_config.as_ref()); - let vector_hnsw_m = vector_hnsw_config.and_then(|hnsw| hnsw.m); - // TODO(strict-mode) check also payload_m if if there is a filter by tenant/principal - if vector_hnsw_m == Some(0) { + let vector_hnsw_m = vector_hnsw_config + .map(|hnsw_config| hnsw_config.m) + .flatten() + .unwrap_or(config.hnsw_config.m); + + let vector_hnsw_payload_m = vector_hnsw_config + .map(|hnsw_config| hnsw_config.payload_m) + .flatten() + .unwrap_or(config.hnsw_config.payload_m.unwrap_or(vector_hnsw_m)); + + // no further check necessary if there is a global HNSW index + if vector_hnsw_m > 0 { + return Ok(()); + } + + // specialized error message if not default vector + let vector_error_label = if using == DEFAULT_VECTOR_NAME { + "" + } else { + &format!(" on '{using}'") + }; + + // check hnsw.payload_m if there is a filter + let uses_multitenant_filter = if let Some(filter) = filter { + filter + .iter_conditions() + .filter_map(|c| c.targeted_key()) + .filter_map(|key| collection.payload_key_index_schema(&key)) + .any(|index_schema| index_schema.is_tenant()) + } else { + false + }; + + if !uses_multitenant_filter { + // HNSW disabled AND no filters return Err(CollectionError::strict_mode( format!( - "Fullscan forbidden on '{using}' – vector indexing is disabled (hnsw_config.m = 0)" + "Request is forbidden{vector_error_label} because global vector indexing is disabled (hnsw_config.m = 0)" ), - "Enable vector indexing or use a prefetch query before rescoring", + "Use tenant-specific filter, enable global vector indexing or enable strict mode `search_allow_exact` option", )); } + + if vector_hnsw_payload_m == 0 { + // HNSW disabled AND no filters + return Err(CollectionError::strict_mode( + format!( + "Request is forbidden{vector_error_label} because vector indexing is disabled (hnsw_config.m = 0 and hnsw_config.payload_m = 0)" + ), + "Enable vector indexing, use a prefetch query with indexed vectors or enable strict mode `search_allow_exact` option", + )); + } + + return Ok(()); } } } @@ -102,7 +148,12 @@ impl StrictModeVerification for CollectionQueryRequest { // check query can perform fullscan when not rescoring if self.prefetch.is_empty() { query - .check_fullscan(&self.using, collection, strict_mode_config) + .check_fullscan( + &self.using, + self.filter.as_ref(), + collection, + strict_mode_config, + ) .await?; } // check for unindexed fields in formula @@ -149,7 +200,12 @@ impl StrictModeVerification for CollectionPrefetch { if let Some(query) = self.query.as_ref() { // check if prefetch can perform a fullscan query - .check_fullscan(&self.using, collection, strict_mode_config) + .check_fullscan( + &self.using, + self.filter.as_ref(), + collection, + strict_mode_config, + ) .await?; // check for unindexed fields in formula query @@ -191,7 +247,12 @@ impl StrictModeVerification for CollectionQueryGroupsRequest { // check query can perform fullscan when not rescoring if self.prefetch.is_empty() { query - .check_fullscan(&self.using, collection, strict_mode_config) + .check_fullscan( + &self.using, + self.filter.as_ref(), + collection, + strict_mode_config, + ) .await?; } // check for unindexed fields in formula diff --git a/lib/segment/src/types.rs b/lib/segment/src/types.rs index 03ca04e443..8de2a2fe50 100644 --- a/lib/segment/src/types.rs +++ b/lib/segment/src/types.rs @@ -2793,6 +2793,17 @@ impl Condition { | Condition::HasVector(_) => 0, } } + + pub fn targeted_key(&self) -> Option { + match self { + Condition::Field(field_condition) => Some(field_condition.key.clone()), + Condition::IsEmpty(is_empty_condition) => Some(is_empty_condition.is_empty.key.clone()), + Condition::IsNull(is_null_condition) => Some(is_null_condition.is_null.key.clone()), + Condition::Nested(nested_condition) => Some(nested_condition.array_key()), + Condition::Filter(filter) => filter.iter_conditions().find_map(|c| c.targeted_key()), + Condition::HasId(_) | Condition::HasVector(_) | Condition::CustomIdChecker(_) => None, + } + } } // The validator crate does not support deriving for enums. diff --git a/tests/openapi/test_strictmode.py b/tests/openapi/test_strictmode.py index 246884524e..616e4c6971 100644 --- a/tests/openapi/test_strictmode.py +++ b/tests/openapi/test_strictmode.py @@ -300,7 +300,7 @@ def test_strict_mode_unindexed_filter_integer_read_validation(collection_name): search_fail = search_request_with_filter() assert "count" in search_fail.json()['status']['error'] assert not search_fail.ok - + def test_strict_mode_unindexed_filter_write_validation(collection_name): def update_request_with_filter(): @@ -1898,7 +1898,6 @@ def test_read_rate_limiter_many_vectors(full_collection_name): }) check_multivector_query_id(should_succeed=False) - # Set strict mode with just enough read_rate_limit, it should succeed set_strict_mode(collection_name, { "enabled": True, @@ -2119,7 +2118,7 @@ def test_strict_mode_full_scan(full_collection_name): } ) assert not response.ok - assert "Fullscan forbidden on 'dense-multi' – vector indexing is disabled (hnsw_config.m = 0). Help: Enable vector indexing or use a prefetch query before rescoring" in response.json()['status']['error'] + assert "Request is forbidden on 'dense-multi' because global vector indexing is disabled (hnsw_config.m = 0). Help: Use tenant-specific filter, enable global vector indexing or enable strict mode `search_allow_exact` option" in response.json()['status']['error'] # sparse vector still works response = request_with_validation( @@ -2173,4 +2172,178 @@ def test_strict_mode_full_scan(full_collection_name): } ) assert not response.ok - assert "Fullscan forbidden on 'dense-multi' – vector indexing is disabled (hnsw_config.m = 0). Help: Enable vector indexing or use a prefetch query before rescoring" in response.json()['status']['error'] + assert "Request is forbidden on 'dense-multi' because global vector indexing is disabled (hnsw_config.m = 0). Help: Use tenant-specific filter, enable global vector indexing or enable strict mode `search_allow_exact` option" in response.json()['status']['error'] + + +def test_strict_mode_full_scan_simple(full_collection_name): + collection_name = full_collection_name + + # Enable strict mode with search_allow_exact + response = request_with_validation( + api='/collections/{collection_name}', + method="PATCH", + path_params={'collection_name': collection_name}, + body={ + "strict_mode_config": { + "enabled": True, + "search_allow_exact": False + }, + } + ) + assert response.ok + + # full scan allowed + response = request_with_validation( + api='/collections/{collection_name}/points/query', + method="POST", + path_params={'collection_name': collection_name}, + body={ + "query": 2, + "using": "dense-text", + "limit": 5 + } + ) + assert response.ok + + # disable HNSW index + response = request_with_validation( + api='/collections/{collection_name}', + method="PATCH", + path_params={'collection_name': collection_name}, + body={ + "hnsw_config": { + "m": 0 + } + } + ) + assert response.ok + + # full scan not allowed + response = request_with_validation( + api='/collections/{collection_name}/points/query', + method="POST", + path_params={'collection_name': collection_name}, + body={ + "query": 2, + "using": "dense-text", + "limit": 5 + } + ) + assert not response.ok + assert response.status_code == 400 + + +def test_strict_mode_multitenant_full_scan(full_collection_name): + collection_name = full_collection_name + + def filtered_query(): + return request_with_validation( + api='/collections/{collection_name}/points/query', + method="POST", + path_params={'collection_name': collection_name}, + body={ + "query": 2, + "filter": { + "must": [ + { + "key": "city", + "match": { + "value": "Berlin" + } + } + ] + }, + "using": "dense-multi", + "limit": 5 + } + ) + + # disable HNSW index + response = request_with_validation( + api='/collections/{collection_name}', + method="PATCH", + path_params={'collection_name': collection_name}, + body={ + "vectors": { + "dense-multi": { + "hnsw_config": { + "m": 0, + "payload_m": 0 + }, + }, + } + } + ) + assert response.ok + + # filtered search allowed + filtered_query().raise_for_status() + + # enable strict mode with search_allow_exact + set_strict_mode(collection_name, { + "enabled": True, + "search_allow_exact": False + }) + + # filtered search not allowed anymore because no HNSW index + response = filtered_query() + assert not response.ok + assert "Request is forbidden on 'dense-multi'" in response.json()['status']['error'] + + # add payload index + request_with_validation( + api='/collections/{collection_name}/index', + method="PUT", + path_params={'collection_name': collection_name}, + query_params={'wait': 'true'}, + body={ + "field_name": "city", + "field_schema": "keyword" + } + ).raise_for_status() + + # still not allowed although we have payload index for the filter + response = filtered_query() + assert not response.ok + assert "Request is forbidden on 'dense-multi'" in response.json()['status']['error'] + + # add multitenant payload index + request_with_validation( + api='/collections/{collection_name}/index', + method="PUT", + path_params={'collection_name': collection_name}, + query_params={'wait': 'true'}, + body={ + "field_name": "city", + "field_schema": { + "type": "keyword", + "is_tenant": True, + } + } + ).raise_for_status() + + # still not allowed although we have a multitenant payload index for the filter + response = filtered_query() + assert not response.ok + assert "Request is forbidden on 'dense-multi'" in response.json()['status']['error'] + + # enabled HNSW payload based index + response = request_with_validation( + api='/collections/{collection_name}', + method="PATCH", + path_params={'collection_name': collection_name}, + body={ + "vectors": { + "dense-multi": { + "hnsw_config": { + "m": 0, + "payload_m": 1 + }, + }, + } + } + ) + assert response.ok + + # finally allowed + filtered_query().raise_for_status()