JingsongLi commented on PR #9909:
URL: https://github.com/apache/paimon/pull/9909#issuecomment-5709392022
The direction looks right — with the index path lacking a final-read filter,
an unevaluable predicate can neither be answered by an empty bitmap nor
ignored, so returning `null` ("cannot decide") and routing those ranges through
the raw search is the only sound fallback. Two implementation notes, both
optional:
**1. Fold the demote into the shared split step**
Every entry point currently does `splitSearchSplits(...)` followed by
`demoteUncoverableIndexSplits(...)` (local / batch / Spark / Flink), with the
ordering contract carried in the javadoc ("Must run after splitSearchSplits and
before the index splits are read"). That contract is convention only: a future
reader that prepares splits elsewhere and forgets the second call silently
regresses to the #9883 behavior. `splitSearchSplits` is already the single
shared entry point — making it an instance method (or wrapping both steps in a
`prepareSplits`) makes the ordering structural and removes the contract from
the javadoc.
**2. The `coveredByRawSplits` / `uncovered` bookkeeping can be dropped**
Every raw read path merges ranges before reading: local/batch via
`withRawSearch` → `rawRowRanges(...)` (`Range.sortAndMergeOverlap(..., true)`),
and Spark/Flink's `readRawSplitsInSpark` / `readRawSplitsInFlink` call the same
helper first. The demote also adds whole split ranges, not set differences. So
"add only the uncovered ranges" and "add all of them" produce the identical
merged union — a range already covered by a raw split disappears into that same
range — and duplicate raw scoring is idempotent (same rowId, same score;
`result.or(rawResult).topK(limit)` dedups). The method collapses to:
```java
protected void prepareSplits(
List<? extends VectorSearchSplit> splits,
List<IndexVectorSearchSplit> indexSplits,
List<RawVectorSearchSplit> rawSplits) {
splitSearchSplits(splits, indexSplits, rawSplits);
scalarPreFilter = null;
if (filter == null || indexSplits.isEmpty()) {
return;
}
RoaringNavigableMap64 matchedRows = scalarMatchedRows(indexSplits); //
probe once
if (matchedRows != null) {
scalarPreFilter = matchedRows; // reused by preFilters
return;
}
List<Range> ranges = new ArrayList<>();
List<IndexFileMeta> files = new ArrayList<>();
for (IndexVectorSearchSplit split : indexSplits) {
ranges.add(new Range(split.rowRangeStart(), split.rowRangeEnd()));
files.addAll(split.scalarIndexFiles());
}
rawSplits.add(new RawVectorSearchSplit(ranges, files,
vectorIndexType(indexSplits)));
indexSplits.clear();
}
```
`vectorIndexType(indexSplits)` needs to stay: `rawSearchIndexer` resolves
the metric from the raw split's indexType. With both steps in one place,
`scalarPreFilter` also becomes an internal detail of this method instead of a
lifecycle documented on the class. The only behavioral delta is that the
demoted split's `scalarIndexFiles` grows from "files of the uncovered splits"
to "files of all splits"; they only feed `rawPreFilter`, which is a sound
superset pre-filter with the exact final-read filter as backstop, so
correctness is unaffected.
--
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]
