LuciferYang commented on issue #9931: URL: https://github.com/apache/paimon/issues/9931#issuecomment-5729873754
This issue and #9855 (merged today) overlap, so I want to settle on one consistent behavior before the next release. Both concern the same situation: a `vector_search` / `hybrid_search` / `full_text_search` with a `WHERE` conjunct that Spark cannot push into Paimon (a UDF, a column-to-column comparison, `id % 2 = 1`, an unresolvable cast). That conjunct stays a Spark residual applied above the search, which has already truncated to the top-K. Post-filtering the top-K can only drop rows, so the result is a subset of the top-K and may be short. It is a silent recall loss: the returned rows are correct, but rows that satisfy the predicate and rank just outside the top-K are never considered. #9855 treats this as expected for full-text search and documents it (`filtered.subsetOf(unfiltered)`, "may be short"). #9931 (PR #9964) treats it as a bug and fails the query fast with a clear error, matching the Flink `vector_search` procedure, which rejects an inexpressible predicate. So two opposite defaults landed for the same case, close together, and my PR's fail-fast currently breaks #9855's full-text test. For reference, this is the standard prefilter/postfilter split. Lance, for example, exposes an explicit `prefilter` flag: postfilter (its default) runs the ANN first and applies the residual afterward, so it may return fewer than k, while prefilter feeds the predicate into the search so the top-K is computed over matching rows. Prefilter is the only mode that returns the true k-nearest-that-match, but it needs the predicate to be evaluable by the engine. For a Spark-only residual that Paimon cannot express, prefilter would degrade to a full scan, so the cheap choices here really are post-filter (short) or fail. Proposal: unify the three search TVFs under one option that selects the residual behavior, instead of letting "did the optimizer manage to translate the predicate" decide it silently: - `post-filter`: the current #9855 behavior, apply the residual above the top-K, may be short. #9855's test keeps its `subsetOf` assertion under this mode. - `fail`: reject the query with a clear message (Flink parity, the strict kNN contract). Same code path for all three search types, so the only real decision is the default. My lean is `post-filter` as the default (it matches #9855 and the common industry default) with `fail` available as an opt-in for callers who want the strict contract, but I would rather align on this with you first. @zhuxiangyi @JingsongLi what do you think the default should be? If that direction sounds right, I will rework #9964 into the unified option across all three TVFs rather than the current fail-only. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
