fix(local): update IDF statistics on deletion (#1427)

* fix(local): update IDF statistics on deletion

* fix: update IDF statistics on deletion, reject writes to absent points, apply the rest

---------

Co-authored-by: George Panchuk <george.panchuk@qdrant.tech>
This commit is contained in:
Kunpeng Xie
2026-09-15 21:42:09 +07:00
committed by GitHub
co-authored by George Panchuk
parent cb23f3f0f2
commit e603c2d5c9
5 changed files with 471 additions and 21 deletions
+116 -19
View File
@@ -231,6 +231,28 @@ class LocalCollection:
for idx in vector.indices:
self.sparse_vectors_idf[vector_name][idx] -= 1
def _drop_idf_contribution(self, idx: int, vector_name: str) -> None:
"""Take point `idx` out of the IDF counters of `vector_name`.
`sparse_vectors_idf` must stay in sync with the corpus `_rescore_idf` measures, which
is every point that is alive and actually has the vector. A point that is already
deleted, point-wise or vector-wise, never counted, so dropping it again is a no-op.
"""
if self.deleted[idx] or self.deleted_per_vector[vector_name][idx]:
return
self._update_idf_remove(self.sparse_vectors[vector_name][idx], vector_name)
def _existing_idx(self, point_id: types.PointId) -> int | None:
"""Internal id of a point the collection still holds, `None` if it holds none.
Deleted points keep their slot so that internal ids stay stable, but the server
answers 404 for them exactly as it does for ids it has never seen.
"""
idx = self.ids.get(point_id)
if idx is None or self.deleted[idx]:
return None
return idx
@staticmethod
def _idf_corpus_of(search_params: types.SearchParams | None) -> types.Filter | None:
"""Corpus filter scoping IDF statistics, if `search_params` asks for a narrowed scope.
@@ -278,6 +300,12 @@ class LocalCollection:
# IDF statistics only take into account points which actually have this sparse vector,
# points missing it are not part of the corpus. `idf_corpus` narrows it down further.
#
# Deleted points leave the corpus right away. `IdfScope.GLOBAL` on the server reads
# the sparse index rather than the live points, so it goes on counting deleted ones
# for as long as they sit in that index; local mode has no index to go stale and
# answers from live statistics immediately. The two agree on `IdfCorpusParams`, which
# is measured over live points either way - that is what the congruence tests pin.
mask = self._payload_and_non_deleted_mask(idf_corpus, vector_name=vector_name)
num_docs = int(np.count_nonzero(mask))
@@ -1850,10 +1878,10 @@ class LocalCollection:
def _vector_by_point_id(
self, vector_name: str, point_id: types.PointId
) -> list[float] | SparseVector | list[list[float]]:
if point_id not in self.ids:
idx = self._existing_idx(point_id)
if idx is None:
raise ValueError(f"Point {point_id} is not found in the collection")
idx = self.ids[point_id]
if vector_name in self.vectors:
vector = self.vectors[vector_name][idx].tolist()
elif vector_name in self.sparse_vectors:
@@ -2550,15 +2578,16 @@ class LocalCollection:
# sparse vectors
for vector_name, _named_vectors in self.sparse_vectors.items():
vector = vectors.get(vector_name)
was_deleted = self.deleted_per_vector[vector_name][idx]
if not was_deleted:
previous_vector = self.sparse_vectors[vector_name][idx]
self._update_idf_remove(previous_vector, vector_name)
# we need to drop it even if the vector exists, because idf is computed over each vector value
# and if the vector has changed - the idf should be recalculated
self._drop_idf_contribution(idx, vector_name)
if vector is not None:
stored_vector = copy_sparse_vector(vector)
self.sparse_vectors[vector_name][idx] = stored_vector
self.deleted_per_vector[vector_name][idx] = 0
# An upsert revives a deleted point, so this counts even when `deleted[idx]`
# is still set - it is cleared at the end of this method.
self._update_idf_append(stored_vector, vector_name)
else:
self.deleted_per_vector[vector_name][idx] = 1
@@ -2838,16 +2867,19 @@ class LocalCollection:
def _apply_named_vectors(self, idx: int, validated: list[tuple[str, Any]]) -> None:
"""Apply already validated named vectors. Call `_validate_named_vectors` first."""
for vector_name, vector_np in validated:
self.deleted_per_vector[vector_name][idx] = 0
if isinstance(vector_np, SparseVector):
old_vector = self.sparse_vectors[vector_name][idx]
self._update_idf_remove(old_vector, vector_name)
# this has to come first: the deletion flag is what tells
# `_drop_idf_contribution` whether this point counts at all, and the stored
# vector is the one it takes back out of the counters
self._drop_idf_contribution(idx, vector_name)
self.deleted_per_vector[vector_name][idx] = 0
stored_vector = copy_sparse_vector(vector_np)
self.sparse_vectors[vector_name][idx] = stored_vector
self._update_idf_append(stored_vector, vector_name)
continue
self.deleted_per_vector[vector_name][idx] = 0
params = self.get_vector_params(vector_name)
if vector_name in self.vectors:
if params.distance == models.Distance.COSINE:
@@ -2866,8 +2898,9 @@ class LocalCollection:
validate_filter(update_filter)
# Same rule as upsert: validate every point in the request before writing any of it.
# The point id itself is looked up in the apply pass, because the server does apply
# the points preceding an unknown id before answering 404.
# The point ids are looked up in the apply pass, because a request naming one the
# collection does not hold is not rejected outright: the server writes every other
# point in it, and only then answers 404 for the first id it could not find.
prepared = [
(
str(point.id) if isinstance(point.id, uuid.UUID) else point.id,
@@ -2880,10 +2913,14 @@ class LocalCollection:
for point in points
]
missing: types.PointId | None = None
for point_id, validated in prepared:
idx = self.ids[point_id]
idx = self._existing_idx(point_id)
if idx is None:
missing = point_id if missing is None else missing
continue
if not self.deleted[idx] and update_filter is not None:
if update_filter is not None:
has_vector = {}
for vector_name, deleted in self.deleted_per_vector.items():
if not deleted[idx]:
@@ -2895,6 +2932,9 @@ class LocalCollection:
self._apply_named_vectors(idx, validated)
self._persist_by_id(point_id)
if missing is not None:
raise KeyError(missing)
def delete_vectors(
self,
vectors: Sequence[str],
@@ -2909,17 +2949,30 @@ class LocalCollection:
# failing. The server errors either way, but whether it deletes first varies.
self._validate_vector_names(vectors)
# Like `update_vectors`, a point the collection does not hold does not stop the rest
# of the request: everything else is deleted, and the 404 comes afterwards.
ids = self._selector_to_ids(selector)
missing: types.PointId | None = None
for point_id in ids:
idx = self.ids[point_id]
idx = self._existing_idx(point_id)
if idx is None:
missing = point_id if missing is None else missing
continue
for vector_name in vectors:
if vector_name in self.sparse_vectors:
self._drop_idf_contribution(idx, vector_name)
self.deleted_per_vector[vector_name][idx] = 1
self._persist_by_id(point_id)
if missing is not None:
raise KeyError(missing)
def _delete_ids(self, ids: list[types.PointId]) -> None:
for point_id in ids:
if point_id in self.ids:
idx = self.ids[point_id]
for vector_name in self.sparse_vectors:
self._drop_idf_contribution(idx, vector_name)
self.deleted[idx] = 1
if self.storage is not None:
@@ -2985,13 +3038,21 @@ class LocalCollection:
),
key: str | None = None,
) -> None:
# A point the collection does not hold does not stop the rest of the request: the
# server applies every other point in it and answers 404 afterwards, naming the first
# id it could not find. Deleted points count as missing - writing to one would also
# persist it, bringing it back the next time the collection is opened.
ids = self._selector_to_ids(selector)
base_payload = to_jsonable_python(payload)
keys: list[JsonPathItem] | None = parse_json_path(key) if key is not None else None
missing: types.PointId | None = None
for point_id in ids:
idx = self.ids[point_id]
idx = self._existing_idx(point_id)
if idx is None:
missing = point_id if missing is None else missing
continue
# Deep-copy per point: a shared object graph here would let a later,
# differently-scoped set_payload(key=...) call mutate other points'
# payloads in place via set_value_by_key's dict.update().
@@ -3004,6 +3065,9 @@ class LocalCollection:
self._persist_by_id(point_id)
if missing is not None:
raise KeyError(missing)
def overwrite_payload(
self,
payload: models.Payload,
@@ -3014,12 +3078,23 @@ class LocalCollection:
| models.PointIdsList
),
) -> None:
# A point the collection does not hold does not stop the rest of the request: the
# server applies every other point in it and answers 404 afterwards, naming the first
# id it could not find. Deleted points count as missing - writing to one would also
# persist it, bringing it back the next time the collection is opened.
ids = self._selector_to_ids(selector)
missing: types.PointId | None = None
for point_id in ids:
idx = self.ids[point_id]
idx = self._existing_idx(point_id)
if idx is None:
missing = point_id if missing is None else missing
continue
self.payload[idx] = deepcopy(to_jsonable_python(payload)) or {}
self._persist_by_id(point_id)
if missing is not None:
raise KeyError(missing)
def delete_payload(
self,
keys: Sequence[str],
@@ -3031,13 +3106,24 @@ class LocalCollection:
),
) -> None:
parsed_keys = [parse_json_path(key) for key in keys]
# A point the collection does not hold does not stop the rest of the request: the
# server applies every other point in it and answers 404 afterwards, naming the first
# id it could not find. Deleted points count as missing - writing to one would also
# persist it, bringing it back the next time the collection is opened.
ids = self._selector_to_ids(selector)
missing: types.PointId | None = None
for point_id in ids:
idx = self.ids[point_id]
idx = self._existing_idx(point_id)
if idx is None:
missing = point_id if missing is None else missing
continue
for parsed_key in parsed_keys:
delete_value_by_key(self.payload[idx], parsed_key)
self._persist_by_id(point_id)
if missing is not None:
raise KeyError(missing)
def clear_payload(
self,
selector: (
@@ -3047,12 +3133,23 @@ class LocalCollection:
| models.PointIdsList
),
) -> None:
# A point the collection does not hold does not stop the rest of the request: the
# server applies every other point in it and answers 404 afterwards, naming the first
# id it could not find. Deleted points count as missing - writing to one would also
# persist it, bringing it back the next time the collection is opened.
ids = self._selector_to_ids(selector)
missing: types.PointId | None = None
for point_id in ids:
idx = self.ids[point_id]
idx = self._existing_idx(point_id)
if idx is None:
missing = point_id if missing is None else missing
continue
self.payload[idx] = {}
self._persist_by_id(point_id)
if missing is not None:
raise KeyError(missing)
def _validate_vector_names(self, vector_names: Sequence[str]) -> None:
for vector_name in vector_names:
if vector_name not in self._all_vectors_keys:
@@ -1,3 +1,6 @@
import pytest
from qdrant_client.http.models import models
from tests.congruence_tests.test_common import (
COLLECTION_NAME,
compare_client_results,
@@ -94,3 +97,89 @@ def test_delete_sparse_points():
remote_client,
lambda c: c.query_points(COLLECTION_NAME, query=vector, using="sparse-image").points,
)
@pytest.mark.parametrize(
"operation",
["set_payload", "overwrite_payload", "delete_payload", "clear_payload", "update_vectors"],
)
@pytest.mark.parametrize("absent", ["unknown", "deleted"])
def test_write_to_an_absent_point(operation: str, absent: str):
"""A point the collection no longer holds is as absent as one it never held.
Deleted points keep their slot locally so that internal ids stay stable, which is why
they have to be excluded explicitly: writing to one would resurrect it in storage. And a
request that names an absent point is not abandoned at that point - the server writes
every other point in it and reports the missing id afterwards, so the ids listed after
the absent one must be written too.
"""
points = generate_fixtures(50)
local_client = init_local()
remote_client = init_remote()
for client in (local_client, remote_client):
init_client(client, points)
client.delete(COLLECTION_NAME, [points[-1].id], wait=True)
missing_id = points[-1].id if absent == "deleted" else max(p.id for p in points) + 1_000
targets = [points[0].id, missing_id, points[1].id]
for client in (local_client, remote_client):
with pytest.raises(Exception):
if operation == "set_payload":
client.set_payload(COLLECTION_NAME, payload={"tag": 1}, points=targets, wait=True)
elif operation == "overwrite_payload":
client.overwrite_payload(
COLLECTION_NAME, payload={"tag": 2}, points=targets, wait=True
)
elif operation == "delete_payload":
client.delete_payload(
COLLECTION_NAME, keys=["rand_digit"], points=targets, wait=True
)
elif operation == "clear_payload":
client.clear_payload(COLLECTION_NAME, points_selector=targets, wait=True)
else:
client.update_vectors(
COLLECTION_NAME,
[
models.PointVectors(
id=point_id, vector={"image": points[2].vector["image"]}
)
for point_id in targets
],
wait=True,
)
compare_collections(local_client, remote_client, 100, attrs=("points_count",))
compare_client_results(
local_client,
remote_client,
lambda c: c.scroll(
COLLECTION_NAME, limit=len(points), with_payload=True, with_vectors=True
)[0],
)
@pytest.mark.parametrize("absent", ["unknown", "deleted"])
def test_recommend_from_an_absent_point(absent: str):
"""Recommending from a deleted point must fail, not fall back on its leftover vector."""
points = generate_fixtures(50)
local_client = init_local()
remote_client = init_remote()
for client in (local_client, remote_client):
init_client(client, points)
client.delete(COLLECTION_NAME, [points[-1].id], wait=True)
missing_id = points[-1].id if absent == "deleted" else max(p.id for p in points) + 1_000
for client in (local_client, remote_client):
with pytest.raises(Exception):
client.query_points(
COLLECTION_NAME,
query=models.RecommendQuery(
recommend=models.RecommendInput(positive=[missing_id])
),
using="image",
limit=5,
)
@@ -1,3 +1,5 @@
import tempfile
import pytest
from qdrant_client.client_base import QdrantBase
@@ -275,8 +277,6 @@ def test_idf_scope_in_prefetch():
def test_search_with_persistence():
import tempfile
fixture_points = generate_sparse_fixtures(
vectors_sizes={"sparse-text": sparse_text_vector_size},
even_sparse=False,
@@ -304,3 +304,149 @@ def test_search_with_persistence():
)
compare_client_results(local_client_2, remote_client, searcher.simple_search_text)
@pytest.mark.parametrize("operation", ["points", "vectors_upsert", "vectors_update"])
def test_idf_after_deletion(operation: str):
"""Deleted points must leave the IDF corpus, and come back when they are re-added.
Only the corpus scope is compared. `IdfScope.GLOBAL` is measured over the sparse index on
the server, which goes on counting deleted points for as long as they sit in that index,
while local mode drops them at once - the two legitimately disagree there.
"""
fixture_points = generate_sparse_fixtures(
vectors_sizes={"sparse-text": sparse_text_vector_size},
even_sparse=False,
)
clients = []
for client in (init_local(), init_remote()):
init_client(
client,
fixture_points,
sparse_vectors_config=sparse_vectors_idf_config,
vectors_config={},
)
clients.append(client)
local_client, remote_client = clients
query = generate_random_sparse_vector(sparse_text_vector_size, density=0.3)
match_all = models.IdfCorpusParams(corpus=models.Filter())
def search(client: QdrantBase) -> list[models.ScoredPoint]:
return client.query_points(
COLLECTION_NAME,
using="sparse-text",
query=query,
search_params=models.SearchParams(idf=match_all),
limit=10,
).points
compare_client_results(local_client, remote_client, search)
whole_corpus = [point.score for point in search(local_client)]
doomed = fixture_points[: len(fixture_points) // 2]
doomed_ids = [point.id for point in doomed]
# repeating a deletion must not take the same document frequencies out twice
for _ in range(2):
for client in clients:
if operation == "points":
client.delete(COLLECTION_NAME, points_selector=doomed_ids, wait=True)
else:
client.delete_vectors(
COLLECTION_NAME, vectors=["sparse-text"], points=doomed_ids, wait=True
)
compare_client_results(local_client, remote_client, search)
assert [
point.score for point in search(local_client)
] != whole_corpus, "deleting half of the corpus did not change the IDF scores"
# re-adding the vectors puts the points back into the corpus, without counting the ones
# that were never taken out a second time
for client in clients:
if operation == "vectors_update":
client.update_vectors(
COLLECTION_NAME,
[models.PointVectors(id=point.id, vector=point.vector) for point in doomed],
wait=True,
)
else:
client.upsert(COLLECTION_NAME, doomed, wait=True)
compare_client_results(local_client, remote_client, search)
assert [point.score for point in search(local_client)] == pytest.approx(
whole_corpus
), "the corpus did not return to its original size"
@pytest.mark.parametrize("operation", ["update_vectors", "delete_vectors"])
@pytest.mark.parametrize("missing", ["unknown", "deleted"])
def test_vector_update_with_a_missing_point(operation: str, missing: str):
"""A point the collection does not hold does not stop the rest of the request.
The server writes every other point in it and answers 404 afterwards, naming the first id
it could not find. A deleted point counts as one of those: it keeps its slot locally so
that internal ids stay stable, but it is gone as far as the request is concerned - and
writing to it would put it back into the IDF corpus, and into storage on the next reopen.
"""
fixture_points = generate_sparse_fixtures(
num=10,
vectors_sizes={"sparse-text": sparse_text_vector_size},
even_sparse=False,
)
replacement = generate_random_sparse_vector(sparse_text_vector_size, density=0.3)
clients = []
for client in (init_local(), init_remote()):
init_client(
client,
fixture_points,
sparse_vectors_config=sparse_vectors_idf_config,
vectors_config={},
)
clients.append(client)
absent = fixture_points[-1].id
for client in clients:
client.delete(COLLECTION_NAME, points_selector=[absent], wait=True)
if missing == "unknown":
absent = max(point.id for point in fixture_points) + 1_000
# the missing id sits in the middle, so anything written after it proves the request was
# not abandoned at the first failure
targets = [fixture_points[0].id, absent, fixture_points[1].id]
for client in clients:
with pytest.raises(Exception):
if operation == "update_vectors":
client.update_vectors(
COLLECTION_NAME,
[
models.PointVectors(id=point_id, vector={"sparse-text": replacement})
for point_id in targets
],
wait=True,
)
else:
client.delete_vectors(
COLLECTION_NAME, vectors=["sparse-text"], points=targets, wait=True
)
def scroll(client: QdrantBase) -> list[models.Record]:
return client.scroll(
COLLECTION_NAME, limit=len(fixture_points), with_vectors=True, with_payload=True
)[0]
compare_client_results(*clients, scroll)
compare_client_results(
*clients,
lambda client: client.query_points(
COLLECTION_NAME,
using="sparse-text",
query=replacement,
search_params=models.SearchParams(idf=models.IdfCorpusParams(corpus=models.Filter())),
limit=10,
).points,
)
+61
View File
@@ -1,3 +1,5 @@
import math
import pytest
from qdrant_client import QdrantClient, models
@@ -294,3 +296,62 @@ def test_fusion_dbsf_score_threshold(qdrant: QdrantClient):
f"Expected 3 points after filtering (threshold 1.0), got {len(result_with_threshold.points)}. "
f"Scores: {[p.score for p in result_no_threshold.points]}"
)
@pytest.mark.parametrize("operation", ["points", "vectors_upsert", "vectors_update"])
def test_idf_statistics_after_deletion(qdrant: QdrantClient, operation: str):
"""Deleting points or their sparse vectors has to take them out of the IDF statistics.
Only the local client can be held to this. `IdfScope.GLOBAL` reads the sparse index on
the server, which goes on counting deleted points for as long as they sit in it, so the
two are expected to differ here and a congruence test cannot pin it down. What can be
pinned down is that the global scope agrees with a corpus selecting every live point,
and that both match the IDF formula.
"""
qdrant.create_collection(
collection_name="test_collection",
vectors_config={},
sparse_vectors_config={"text": models.SparseVectorParams(modifier=models.Modifier.IDF)},
)
vector = models.SparseVector(indices=[0], values=[1.0])
qdrant.upsert(
collection_name="test_collection",
points=[models.PointStruct(id=i, vector={"text": vector}) for i in range(3)]
# a point without the sparse vector is not part of the corpus to begin with
+ [models.PointStruct(id=3, vector={})],
)
def assert_scores(corpus_size: int):
# ((n - df + 0.5) / (df + 0.5) + 1).ln() with every document holding the one term
expected = math.log((corpus_size + 1) / (corpus_size + 0.5))
for idf in (models.IdfScope.GLOBAL, models.IdfCorpusParams(corpus=models.Filter())):
points = qdrant.query_points(
collection_name="test_collection",
using="text",
query=vector,
search_params=models.SearchParams(idf=idf),
).points
assert len(points) == corpus_size
assert [point.score for point in points] == pytest.approx([expected] * corpus_size)
assert_scores(3)
# repeating a deletion must not take the same document frequencies out twice
for _ in range(2):
if operation == "points":
qdrant.delete("test_collection", points_selector=[1, 2, 3])
else:
qdrant.delete_vectors("test_collection", vectors=["text"], points=[1, 2, 3])
assert_scores(1)
if operation == "vectors_update":
qdrant.update_vectors(
"test_collection",
[models.PointVectors(id=i, vector={"text": vector}) for i in (1, 2)],
)
else:
qdrant.upsert(
"test_collection",
[models.PointStruct(id=i, vector={"text": vector}) for i in (1, 2)],
)
assert_scores(3)
+57
View File
@@ -201,3 +201,60 @@ def test_update_persistence():
"not_important": "missing",
}
client.close()
@pytest.mark.parametrize("operation", ["points", "vectors"])
def test_idf_persistence_after_deletion(operation: str):
"""Reopening a collection must not move the scores a deletion left behind.
IDF statistics are maintained incrementally while the client is open and rebuilt from the
stored points when it reopens. A deletion that misses the live statistics therefore scores
one way before a restart and another way after.
"""
collection_name = "idf_persistence"
vector = rest.SparseVector(indices=[0], values=[1.0])
def scores(client: QdrantClient) -> list[float]:
collected = []
for idf in (rest.IdfScope.GLOBAL, rest.IdfCorpusParams(corpus=rest.Filter())):
points = client.query_points(
collection_name,
using="text",
query=vector,
search_params=rest.SearchParams(idf=idf),
).points
collected.append([point.score for point in points])
assert collected[0] == pytest.approx(
collected[1]
), "the global scope disagrees with a corpus of every live point"
return collected[0]
with tempfile.TemporaryDirectory() as tmpdir:
client = QdrantClient(path=tmpdir)
client.create_collection(
collection_name,
vectors_config={},
sparse_vectors_config={"text": rest.SparseVectorParams(modifier=rest.Modifier.IDF)},
)
client.upsert(
collection_name,
[rest.PointStruct(id=i, vector={"text": vector}) for i in range(4)],
)
if operation == "points":
client.delete(collection_name, points_selector=[1, 2, 3])
else:
client.delete_vectors(collection_name, vectors=["text"], points=[1, 2, 3])
before_reopen = scores(client)
assert len(before_reopen) == 1
client.close()
client = QdrantClient(path=tmpdir)
assert scores(client) == pytest.approx(
before_reopen
), "reopening the collection changed the IDF scores"
# deleting the vectors leaves the points themselves in place
surviving = [0] if operation == "points" else [0, 1, 2, 3]
assert [point.id for point in client.scroll(collection_name, limit=10)[0]] == surviving
client.close()