JingsongLi commented on code in PR #672:
URL: https://github.com/apache/paimon-rust/pull/672#discussion_r3712877584
##########
crates/paimon/src/table/global_index_scanner.rs:
##########
@@ -722,35 +823,104 @@ impl GlobalIndexScanner {
Some(probe) => Some(probe.enter().await),
None => None,
};
+ let remaining = limit.saturating_sub(all_row_ids.len() as
usize);
let result = self
- .query_entry(entry, data_type, between, &plan,
effective_predicates)
+ .query_entry(
+ entry,
+ data_type,
+ between,
+ &plan,
+ effective_predicates,
+ Some(remaining),
+ )
.await?;
- Ok((entry.row_range_start, result))
- });
- let all_row_ids = try_fold_bounded(
- futures,
- self.global_index_thread_num,
- RoaringTreemap::new(),
- |all_row_ids, (row_range_start, file_result)| {
- if let Some(bitmap) = file_result {
+ if let Some(bitmap) = result {
for row_id in bitmap.iter() {
- all_row_ids.insert(row_id + row_range_start as u64);
+ all_row_ids.insert(row_id + entry.row_range_start as
u64);
}
}
- },
- )
- .await?;
+ if all_row_ids.len() >= limit as u64 {
Review Comment:
[P1] Do not stop on conservative bitmap candidates
This early stop assumes every returned row ID is an exact predicate match.
However, `BitmapGlobalIndexReader::query` deliberately returns `is_not_null()`
as a conservative superset for Float/Double `NotEq`, `NotIn`, range, and
`Between` operators, with exact filtering deferred to the data reader. With
`LIMIT 1`, a nonmatching candidate from the newest shard can stop planning; the
residual filter then removes it, while an older shard or unindexed tail
containing a real match was never included. Please enable early stop only for
index evaluations proven exact, at minimum excluding bitmap floating
residual-sensitive operators.
##########
crates/paimon/src/table/global_index_scanner.rs:
##########
@@ -722,35 +823,104 @@ impl GlobalIndexScanner {
Some(probe) => Some(probe.enter().await),
None => None,
};
+ let remaining = limit.saturating_sub(all_row_ids.len() as
usize);
let result = self
- .query_entry(entry, data_type, between, &plan,
effective_predicates)
+ .query_entry(
+ entry,
+ data_type,
+ between,
+ &plan,
+ effective_predicates,
+ Some(remaining),
Review Comment:
[P1] Apply the shard limit after global row-ID deduplication
Different global-index identities and index types may legally cover
overlapping row ranges. Capping this shard to the global `remaining` count
before offsetting and inserting into `all_row_ids` allows all returned IDs to
be duplicates from an earlier shard. The limited reader can then stop before
later unique matches; index coverage still marks that range as indexed, so raw
fallback does not recover them. Please disable per-shard limiting when selected
coverage overlaps, or continue scanning until the shard contributes `remaining`
new global row IDs. A mixed BTree/bitmap partial-overlap test would expose the
underfilled LIMIT.
##########
crates/paimon/src/table/global_index_scanner.rs:
##########
@@ -929,8 +1100,9 @@ impl GlobalIndexScanner {
plan.allow_btree = plan.selected_btree > 0
&& btree_valid
- && self.btree_fallback_scan_max_size > 0
- && btree_total <= self.btree_fallback_scan_max_size;
+ && (allow_large_btree
Review Comment:
[P2] Preserve the configured BTree fallback-scan bound
`allow_large_btree` bypasses `btree-index.fallback-scan-max-size`, including
an explicit value of `0`. The limited scan caps matching row IDs, not bytes,
blocks, or entries read, so a sparse or no-match complex LIKE can still scan
every block of arbitrarily large selected indexes during planning. Please
exempt only the cheap preferred point/prefix probe; if that probe does not fill
the limit, reapply the configured size bound and fall back normally, or enforce
a real I/O budget.
--
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]
