Strict mode allows fullscan for multitenant payload index (#6498)

* Strict mode allows fullscan for multitenant payload index

* different error for missing payload index in multitenant case

* handle payload_m

* add simple test

* handle hnsw.m not set

* add another simple test

* fix test

* fallback to global HSNW config

* new error status code

* do not block on global HNSW and improver error reporting

* clearer error reporting

* review fixes

* fix test error messages

---------

Co-authored-by: generall <andrey@vasnetsov.com>
This commit is contained in:
Arnaud Gourlay
2025-05-22 10:42:47 +02:00
committed by generall
parent 5e7d9aa4fc
commit dd97253fe9
3 changed files with 258 additions and 13 deletions

View File

@@ -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

View File

@@ -2793,6 +2793,17 @@ impl Condition {
| Condition::HasVector(_) => 0,
}
}
pub fn targeted_key(&self) -> Option<PayloadKeyType> {
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.

View File

@@ -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()