fix(datafusion): bound global-index filter planning under limit - #672
fix(datafusion): bound global-index filter planning under limit#672XiaoHongbo-Hope wants to merge 5 commits into
Conversation
dc8848b to
a4098c8
Compare
| matches!( | ||
| predicate, | ||
| Predicate::Leaf { | ||
| op: PredicateOperator::Eq | PredicateOperator::Like, |
There was a problem hiding this comment.
[P1] Keep filtered row counts inexact
Marking data equality and LIKE predicates as exact is valid for filter execution, but this same classification becomes PaimonTableScan.filter_exact. With no limit, partition_statistics then publishes sum(split.merged_row_count()) as Precision::Exact, even though those counts are computed before this data predicate. DataFusion AggregateStatistics can therefore rewrite SELECT COUNT(*) FROM t WHERE name = "alice" or a residual LIKE to the unfiltered file count without executing the scan. Please separate "the connector enforces this filter exactly" from "the pre-filter split row count is exact", or keep num_rows inexact whenever data predicates are present. A COUNT(*) regression for equality and residual LIKE would catch this.
There was a problem hiding this comment.
Fixed in 0f1b057. Filter execution exactness is now separate from statistics exactness: PaimonTableScan records whether the normalized filter contains data predicates and keeps num_rows Inexact whenever it does. Added equality and exact-LIKE COUNT() regressions; partition-only COUNT() remains exact.
| }, | ||
| ) | ||
| .await?; | ||
| if all_row_ids.len() >= limit as u64 { |
There was a problem hiding this 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.
There was a problem hiding this comment.
Fixed in 0f1b057. The planning LIMIT is now enabled only for one exact Eq/Like data predicate; range and compound predicates receive no index early-stop limit. Added a guard regression covering the floating-range case.
| between, | ||
| &plan, | ||
| effective_predicates, | ||
| Some(remaining), |
There was a problem hiding this 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.
There was a problem hiding this comment.
Fixed in 0f1b057. Selected shard ranges are checked for overlap. When they overlap, reader-local limiting is disabled and aggregation continues through each shard until the global de-duplicated row-ID set reaches LIMIT. Added a partial-overlap mixed BTree/bitmap regression.
| && btree_valid | ||
| && self.btree_fallback_scan_max_size > 0 | ||
| && btree_total <= self.btree_fallback_scan_max_size; | ||
| && (allow_large_btree |
There was a problem hiding this 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.
There was a problem hiding this comment.
Fixed in 0f1b057. Only the cheap preferred point/prefix probe may run above the BTree fallback byte budget. If it does not fill LIMIT, the configured bound is re-applied; an over-budget complex LIKE returns unsupported and falls back to the normal data scan. Added regressions for both the bounded fallback and successful preferred-probe paths.
JingsongLi
left a comment
There was a problem hiding this comment.
Two remaining issues found in the follow-up review:
| matches!( | ||
| predicate, | ||
| Predicate::Leaf { | ||
| op: PredicateOperator::Eq | PredicateOperator::Like, |
There was a problem hiding this comment.
[P1] Enable exact Eq/LIKE pushdown for unpartitioned tables
This new exact classification is unreachable when partition_keys is empty because the function returns false above. DataFusion therefore keeps the Inexact filter above TableScan; PushDownLimit cannot commute the limit through that filter, so PaimonTableProvider::scan receives no limit and safe_global_index_limit is never reached. This leaves the documented unpartitioned row-tracking/data-evolution global-index tables exposed to the same unbounded index planning and OOM risk that this PR is intended to fix. Please classify these exact data leaves even when there are no partition keys, and add an unpartitioned physical-plan regression asserting that PaimonTableScan receives the limit.
There was a problem hiding this comment.
Fixed in b33774b. Exact Eq/LIKE data leaves are now classified for unpartitioned tables as well; the empty-partition-key short-circuit was removed. Added a self-contained DataFusion physical-plan regression that creates an unpartitioned Paimon table, verifies PaimonTableScan receives limit=1, and checks the query result.
| let exact_single_predicate = matches!( | ||
| data_predicates, | ||
| [Predicate::Leaf { | ||
| op: PredicateOperator::Eq | PredicateOperator::Like, |
There was a problem hiding this comment.
[P2] Exclude bitmap NaN equality from index early-stop
This treats every Eq predicate as an exact index match, but bitmap float/double keys canonicalize all NaN payloads into one dictionary key. A limited lookup can therefore return an earlier NaN with a different payload and truncate a later row whose payload matches the literal. The exact Arrow residual uses IEEE totalOrder equality, drops the earlier candidate, and cannot recover the truncated match, so WHERE f = <NaN> LIMIT 1 can return zero rows despite a match. Please disable early-stop for Float/Double equality when bitmap indexes may participate, or align the index and residual NaN equality semantics, and add a multi-payload NaN + LIMIT regression.
There was a problem hiding this comment.
Fixed in b33774b. Limited planning now declines Float/Double equality early-stop whenever any selected global-index shard uses Bitmap, so execution falls back to the normal scan instead of truncating canonicalized NaN candidates. Extended the Float/Double multi-payload NaN regressions (negative, positive, and canonical payloads) with LIMIT 1 to verify the scanner returns unsupported for early-stop.
b33774b to
769ae5a
Compare
Purpose
Follow-up to #666. Global-index Eq/LIKE planning could retain or decode far more row IDs than an unordered LIMIT needs and OOM an 8 GiB worker.
Changes
global-index.query-max-memory(256 MiB by default) on encoded and decompressed BTree/Bitmap blocks; oversized index work falls back to the data scan.Validation