fix(fts): support list columns in post-filter path - #8185
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75566f0bc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| iter_str_array(elements.as_ref()) | ||
| .flatten() | ||
| .any(|element| has_query_token(element, tokenizer, query_tokens)) |
There was a problem hiding this comment.
When a list post-filter receives a multi-term MatchQuery with Operator::And, this predicate accepts the row as soon as any element contains either query token; for example, docs = ["alpha"] matches the query alpha beta. The indexed path applies query.operator and requires both terms, so vector searches using prefilter(false) can now return false positives for list columns. Accumulate matches across the entire list row and evaluate them according to the query's operator instead of using any over individual token hits.
Useful? React with 👍 / 👎.
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The list post-filter needs to preserve indexed row-document semantics for the loaded tokenizer and supported MatchQuery options. The current predicate is only correct for exact single-term/default-OR cases, so otherwise equivalent indexed and vector-post-filter queries can return different rows.
A viable revision should evaluate each parent list as the same document used by indexing, honor AND across the whole row, and add parity coverage for a non-default tokenizer. Unsupported query modes should fail clearly instead of silently changing results.
| let is_match = elements.is_some_and(|elements| { | ||
| iter_str_array(elements.as_ref()) | ||
| .flatten() | ||
| .any(|element| has_query_token(element, tokenizer, query_tokens)) |
There was a problem hiding this comment.
This evaluates each list element as an independent document and accepts the row on the first matching query token. That violates two supported contracts: Operator::And is degraded to OR, and non-default tokenizers do not see the space-joined row document used by indexing. This causes false positives and can cause false negatives. Build the same row-document tokenization boundary as the index and honor the query operator, or explicitly reject modes the post-filter cannot preserve.
Reproducer
I extended test_fts_list_postfilter_vector_search and ran cargo test -p lance --lib test_fts_list_postfilter_vector_search -- --nocapture against this head.
- Querying target missing with Operator::And expected 0 rows but returned ids [0, 3] for both List and LargeList.
- After rebuilding the FTS index with InvertedIndexParams::default().base_tokenizer("raw".to_owned()), indexed target search returned 1 row while the vector post-filter returned ids [0, 3] (2 rows), again for both outer list types.
The unmodified single-term/default-tokenizer test still passes.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The post-filter still needs to construct exactly the same row document as the index writer. Empty-string list elements currently violate that invariant under tokenizers that preserve whitespace, so indexed and vector-post-filter searches can return different rows.
A viable revision should share the canonical list materialization logic (or exactly reproduce its separator rule) and add indexed/post-filter parity coverage with a leading empty element and the raw tokenizer.
| let is_match = elements.is_some_and(|elements| { | ||
| // IndexWorker uses the same separator to turn one list row into one document. | ||
| // Tokenizing elements independently changes semantics for tokenizers such as raw. | ||
| let document = iter_str_array(elements.as_ref()).flatten().join(" "); |
There was a problem hiding this comment.
This materialization differs from the index writer for a leading empty element: ["", "target"] becomes " target" here, while materialize_string_list produces "target". With the raw tokenizer, a target query therefore misses in the post-filter even though indexed search finds the row. Reuse the canonical materializer or exactly mirror its separator rule.
Reproducer
I added a leading empty string before the existing null and target elements in test_fts_list_postfilter_vector_search, then ran:
cargo test -p lance --lib test_fts_list_postfilter_vector_search -- --nocapture
The indexed assertion returned [3], but the post-filter returned [] instead of [3] for both List and LargeList. The test failed in both cases.
1 participant
Summary
ListandLargeListstring support toFlatMatchFilterExecOperator::OrandOperator::Andacross the whole rowrawtokenizersProblem
FTS indexing and direct/flat search accept
list<string>andlarge_list<string>columns, butFlatMatchFilterExeconly dispatchedUtf8andLargeUtf8. As a result, the same match query failed at runtime when used as a post-filter, such as after vector search withprefilter(false).The initial list dispatch also exposed two semantic differences from the indexed path: treating each element as an independent document changes non-default tokenizer behavior, and checking for any query token degrades
Operator::Andto OR.Implementation
The post-filter now dispatches
ListandLargeListarrays to a shared offset-generic helper. Each parent list is materialized with the same space separator used byIndexWorker, then evaluated as one document with the tokenizer loaded from the FTS index. The match predicate groups query tokens by token position so AND queries require every query position while still allowing tokenizer alternatives at the same position.Null lists, empty lists, and lists containing only null elements remain non-matches. Unsupported column/list item types return contextual errors. Fuzzy match queries are not currently reproducible in this post-filter because they require index-dictionary expansion, so they now return a clear
NotSupportederror.The integration test covers both outer list offset types and verifies:
rawtokenizer preserves the same row-document boundaryFlatMatchFilterExecwrapsANNSubIndexFixes #7662.
Validation
cargo fmt --all -- --checkcargo clippy --all --tests --benches -- -D warningscargo test -p lance --lib test_fts_list_postfilter_vector_search -- --nocapturecargo test -p lance --lib test_flat_match_filter_find_matches_large_utf8 -- --nocaptureThanks to @BubbleCal for the clear issue report and suggested integration coverage.