mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-11-12 17:20:48 +00:00
Fix: Allow AI chat filters over more than 32700 documents
Non-superusers chatting across a large document set got "couldn't find any content" once they could see more than 32700 documents. Lists above that size are now bound as a single JSON array and expanded with json_each(), which has no parameter limit.
This commit is contained in:
1 parent
a1d76f3a19
commit
119a779629
4 files changed
+38
-55
No files matched your search
@@ -129,11 +129,9 @@ def _stream_chat_with_documents(
|
||||
|
||||
config = AIConfig()
|
||||
if unrestricted:
|
||||
# Exclude trashed ids (usually few) instead of an IN filter over the
|
||||
# full permitted set, which risks the vector store's bound parameter
|
||||
# limit (_MAX_IN_VALUES) on large installs. Trashed documents stay
|
||||
# indexed until permanent deletion (delete_document_from_llm_index
|
||||
# hangs off post_delete, not trash), so must be excluded explicitly.
|
||||
# Exclude trashed ids (usually few) rather than send every permitted
|
||||
# id on each query. Trashed documents stay indexed until permanent
|
||||
# deletion (post_delete, not trash), so must be excluded explicitly.
|
||||
trashed_ids = Document.deleted_objects.values_list("pk", flat=True)
|
||||
filters = exclude_document_ids_filter(str(pk) for pk in trashed_ids)
|
||||
else:
|
||||
|
||||
@@ -412,10 +412,8 @@ class TestStreamChatRetrieval:
|
||||
WHEN:
|
||||
- stream_chat_with_documents is called with unrestricted=True
|
||||
THEN:
|
||||
- The retriever receives a NOT IN filter excluding zero ids, so
|
||||
the whole index is effectively searched -- and no IN-list is
|
||||
built from the full permitted set, which is what risks the
|
||||
vector store's safety limit on large installs
|
||||
- The retriever receives a NOT IN filter excluding zero ids
|
||||
rather than an IN list of every permitted id
|
||||
"""
|
||||
document = DocumentFactory.create(content="indexed document content")
|
||||
indexing.llm_index_add_or_update_document(document)
|
||||
|
||||
@@ -18,7 +18,7 @@ from paperless_ai.migrations import Migration
|
||||
from paperless_ai.migrations import m0001_v1_to_v2
|
||||
from paperless_ai.tables import DocumentChunksTable
|
||||
from paperless_ai.tables import DocumentMetaTable
|
||||
from paperless_ai.vector_store import _MAX_IN_VALUES
|
||||
from paperless_ai.vector_store import _MAX_INLINE_IN_VALUES
|
||||
from paperless_ai.vector_store import DB_FILENAME
|
||||
from paperless_ai.vector_store import DB_PAGE_SIZE
|
||||
from paperless_ai.vector_store import DEFAULT_TABLE_NAME
|
||||
@@ -354,37 +354,39 @@ class TestBuildWhere:
|
||||
assert params == []
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"build_filter",
|
||||
[_in_filter, _nin_filter],
|
||||
ids=["in", "nin"],
|
||||
("build_filter", "expected_ids"),
|
||||
[
|
||||
pytest.param(_in_filter, ["a", "c"], id="in"),
|
||||
pytest.param(_nin_filter, ["b"], id="nin"),
|
||||
],
|
||||
)
|
||||
def test_fails_closed_when_filter_exceeds_max_values(
|
||||
def test_query_with_oversized_filter_scopes_results(
|
||||
self,
|
||||
store: PaperlessSqliteVecVectorStore,
|
||||
build_filter: Callable[[list[str]], MetadataFilters],
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
expected_ids: list[str],
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- An IN or NOT IN filter with more values than _MAX_IN_VALUES
|
||||
(SQLite's own bound-parameter limit is 32766; this guard sits
|
||||
below that with headroom for the query's other bound parameters)
|
||||
- A store holding documents 1, 2 and 40000
|
||||
- An IN or NOT IN filter naming documents 1 and 40000, padded
|
||||
past _MAX_INLINE_IN_VALUES with unindexed ids
|
||||
WHEN:
|
||||
- _build_where() translates it to SQL
|
||||
- The store is queried and get_nodes() is called with that filter
|
||||
THEN:
|
||||
- It fails closed ("1 = 0", no params) instead of building a
|
||||
clause SQLite would reject, and logs a warning -- this filter
|
||||
scopes document access, so refusing to build it must never
|
||||
widen the scope to "everything" by accident. Failing open on
|
||||
a NOT IN would surface exactly the excluded rows
|
||||
- Both return exactly the documents the filter allows
|
||||
"""
|
||||
oversized = build_filter([str(i) for i in range(_MAX_IN_VALUES + 1)])
|
||||
store.add(
|
||||
[make_node("a", 1), make_node("b", 2), make_node("c", 40000)],
|
||||
)
|
||||
padding = range(100, 100 + _MAX_INLINE_IN_VALUES)
|
||||
filters = build_filter([str(i) for i in (1, 40000, *padding)])
|
||||
|
||||
with caplog.at_level("WARNING"):
|
||||
where, params = _build_where(oversized)
|
||||
result = _query(store, [0.0] * DIM, top_k=5, filters=filters)
|
||||
nodes = store.get_nodes(filters=filters)
|
||||
|
||||
assert where == "(1 = 0)"
|
||||
assert params == []
|
||||
assert "document_id" in caplog.text
|
||||
assert sorted(result.ids) == expected_ids
|
||||
assert sorted(n.node_id for n in nodes) == expected_ids
|
||||
|
||||
def test_query_with_untranslatable_filter_returns_no_rows(
|
||||
self,
|
||||
|
||||
@@ -89,15 +89,10 @@ class _Row(NamedTuple):
|
||||
embedding: bytes
|
||||
|
||||
|
||||
# _build_where(): the largest IN value list translated into bound SQL
|
||||
# parameters. SQLite's own hard limit (SQLITE_MAX_VARIABLE_NUMBER) is 32766
|
||||
# by default; this leaves headroom below that for the query's other bound
|
||||
# parameters (the embedding blob, k, and any NE clause) and for the limit
|
||||
# itself to move. An IN filter this large should not happen in practice --
|
||||
# callers are expected to pass None (no filter) rather than every id when
|
||||
# the filter would not actually narrow anything -- so this is a guard
|
||||
# against a future regression, not a normal code path.
|
||||
_MAX_IN_VALUES = 32700
|
||||
# _build_where(): the largest IN / NOT IN list bound as one parameter per value,
|
||||
# leaving headroom under SQLite's 32766-variable limit for the query's other
|
||||
# parameters. Longer lists are bound as a single JSON array via json_each().
|
||||
_MAX_INLINE_IN_VALUES = 32700
|
||||
|
||||
|
||||
def _pack(embedding: Sequence[float]) -> bytes:
|
||||
@@ -120,7 +115,7 @@ def _vec0_params(rows: list[_Row]) -> list[tuple[str, int, str, bytes]]:
|
||||
return [(r.chunk_id, r.document_id, r.node_content, r.embedding) for r in rows]
|
||||
|
||||
|
||||
def _build_where(filters: MetadataFilters | None) -> tuple[str, list[int]]:
|
||||
def _build_where(filters: MetadataFilters | None) -> tuple[str, list[int | str]]:
|
||||
"""Translate the EQ / IN / NIN / NE filters we use into a parameterized
|
||||
SQL clause on vec0 metadata columns. Returns ("", []) when there is
|
||||
nothing to filter. document_id is vec0's only filterable column and is
|
||||
@@ -132,7 +127,7 @@ def _build_where(filters: MetadataFilters | None) -> tuple[str, list[int]]:
|
||||
if filters is None or not filters.filters:
|
||||
return "", []
|
||||
clauses: list[str] = []
|
||||
params: list[int] = []
|
||||
params: list[int | str] = []
|
||||
for f in filters.filters:
|
||||
# filters.filters is Union[MetadataFilter, ExactMatchFilter, MetadataFilters];
|
||||
# we only build MetadataFilter entries, so skip anything else at runtime.
|
||||
@@ -149,21 +144,11 @@ def _build_where(filters: MetadataFilters | None) -> tuple[str, list[int]]:
|
||||
# excludes nothing, so it matches everything.
|
||||
clauses.append("1 = 0" if is_in else "1 = 1")
|
||||
continue
|
||||
if len(values) > _MAX_IN_VALUES:
|
||||
# Refuse rather than risk SQLite's own bound-parameter limit
|
||||
# ("too many SQL variables"): a list this large must match no
|
||||
# rows, never widen the scope to "everything" -- true for
|
||||
# NOT IN too, where failing open would surface every
|
||||
# excluded row.
|
||||
logger.warning(
|
||||
"Refusing to build a %s filter on %r with %d values "
|
||||
"(over the %d-value safety limit); returning no rows.",
|
||||
sql_op,
|
||||
f.key,
|
||||
len(values),
|
||||
_MAX_IN_VALUES,
|
||||
if len(values) > _MAX_INLINE_IN_VALUES:
|
||||
clauses.append(
|
||||
f"{f.key} {sql_op} (SELECT value FROM json_each(?))",
|
||||
)
|
||||
clauses.append("1 = 0")
|
||||
params.append(json.dumps(values))
|
||||
continue
|
||||
placeholders = ",".join("?" for _ in values)
|
||||
clauses.append(f"{f.key} {sql_op} ({placeholders})")
|
||||
|
||||
Reference in new issue
Block a user