HIGH: kvrocks.py — not/nt negation silently returns empty + N+1 smembers round trips in scan_iter loops

[t0kubetsu]

Summary

Two correctness/performance bugs in kvrocks.py introduced or exposed by the v0.2606.0 wildcard-search refactor (which fixed the r.keys() blocking from #90).

Finding 1 — not/nt suffix accepted but silently returns empty results (kvrocks.py:662, 735)

Both get_uids_by_criteria and get_uids_by_criteria_scoped include "not" and "nt" in the suffix branch that drives the scan_iter loop. The loop body only adds to matching_uids for like/begin conditions — "not" is never handled. After the loop:

partial_result = partial_result.intersection(matching_uids)  # matching_uids is always set()

Any field.not:value query silently returns zero results. Users get no error and no hint that negation is unimplemented. tagrules.py implements not correctly via _document_field_matches — the Kvrocks path should either match or raise ValueError("not/nt modifier not yet implemented").

Finding 2 — N+1 smembers calls inside scan_iter loop (kvrocks.py:472, 664, 736)

For every key returned by scan_iter, a synchronous self.r.smembers(key) is issued inside the loop body. On a large index this produces thousands of serial Redis round trips per search request. The same pattern appears in _get_uids_for_http_headval (line 472).

Fix: collect matching keys first, then issue all smembers calls in a single pipeline:

matching_keys = [k for k in self.r.scan_iter(f"{base_field}:*", count=1000)
                 if value_matches(k, suffix, value)]
if matching_keys:
    pipe = self.r.pipeline(transaction=False)
    for k in matching_keys:
        pipe.smembers(k)
    for members in pipe.execute():
        matching_uids.update(members.intersection(partial_result))

Also: the two scan_iter calls at lines 664 and 736 are missing the count=1000 hint used everywhere else in the file.

File

webapp/app/utils/kvrocks.py

Relation to #90

The r.keys() blocking was fixed in v0.2606.0. The N+1 pattern above is a new issue in the replacement scan_iter loops.

[Th4nat0s]

Will be implemented later diferently.

[t0kubetsu]

Understood on not/nt — leaving that for the future refactor.

The N+1 smembers part of this issue is independent and has been extracted into a focused PR: #161 ("Fix kvrocks: pipeline smembers to eliminate N+1 Redis round trips"). That PR contains no not/nt changes — only the pipeline batching fix for get_uids_by_criteria, get_uids_by_criteria_scoped, and _get_uids_for_http_headval.