feat: add predicate pushdown for Vortex format - #511
Conversation
Signed-off-by: Ted Xu <ted.xu@zilliz.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tedxu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
See milvus-io#512 Implement predicate pushdown for Vortex files using sqlparser-rs to parse SQL WHERE-clause predicates into native Vortex expressions. - Add sqlparser-based predicate parser in Rust bridge that converts SQL predicates to vortex::expr::Expression (supports comparisons, AND/OR/NOT, IN lists, all-digit quoted column names) - Add expr::parse_predicate() to CXX bridge for one-time parsing - Wire predicate string through Reader -> PackedRecordBatchReader -> ColumnGroupReader -> FormatReader::set_predicate() - VortexFormatReader parses predicate once in set_predicate(), applies via ScanBuilder::WithFilter() by-ref on each read() - For single CG: use get_chunk() per chunk to avoid chunk-slicing crash when predicate filtering changes row counts - For multi CG: log warning and ignore predicate (row alignment requires row-index tracking, not yet implemented) - Parquet/Lance/Iceberg: predicate is a no-op (default set_predicate) - Handle empty filtered results (0 chunks from Vortex scan) - Add --predicate and --verbose flags to loon read command - Add predicate pushdown benchmark (sorted vs unsorted, int64 vs string, with filesystem I/O metrics) - Add benchmark results doc with analysis Signed-off-by: Ted Xu <ted.xu@zilliz.com>
0785c1d to
261f62a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #511 +/- ##
==========================================
+ Coverage 72.61% 74.56% +1.94%
==========================================
Files 148 149 +1
Lines 13794 13873 +79
Branches 2053 2078 +25
==========================================
+ Hits 10017 10344 +327
+ Misses 3777 3529 -248
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| */ | ||
| [[nodiscard]] arrow::Result<std::unique_ptr<ChunkReader>> get_chunk_reader( | ||
| int64_t column_group_index, | ||
| const std::shared_ptr<std::vector<std::string>>& needed_columns = nullptr) const override { |
There was a problem hiding this comment.
also add const std::string& /*predicate*/ ?
There was a problem hiding this comment.
The chunk reader, per our previous design, does not support predicate. Filtering on chunks may break the caching preconditions.
|
|
||
| // Predicate pushdown only supported for single column group. | ||
| // Multi-CG filtering requires row-index tracking for cross-CG alignment. | ||
| if (!predicate_.empty() && column_groups_.size() > 1) { |
There was a problem hiding this comment.
We can support Multi-CG filtering if lower level reader return the mask. Still can align the rows
| } | ||
|
|
||
| if (parsed_predicate_) { | ||
| scan_builder.WithFilter(*parsed_predicate_); |
There was a problem hiding this comment.
I think the main issue with the current PR is that it performs full filtering rather than sparse filtering. The performance of full filtering is much worse than sparse filtering. My understanding is that, for “filter pushdown,” the storage layer only needs to provide sparse filtering capability at this stage.
There was a problem hiding this comment.
Performing two complete filter, especially with a low filter rate, will reduce the end-to-end performance.
There was a problem hiding this comment.
Vortex's with_filter() couples zone-map pruning with row-level evaluation in the patched-in version; there is no prune-only knob today. Refreshed benchmark numbers (stats now default-on after this PR) still show net wins on selective sorted data, e.g. 75% I/O reduction and 2.2× speedup at 10% selectivity vs the unsorted baseline. If/when Vortex exposes a prune-only path, we'll switch. Tracking as a follow-up in #524.
Drop the SELECT-wrap hack in the Rust parser and parse the SQL expression directly. Reject trailing tokens after the expression so things like "1=1; DROP TABLE x" no longer silently truncate. Enable Vortex writer statistics by default so zone-map pruning is exercised under the default writer configuration. Make the predicate parser schema-aware. Syntax errors, unknown columns and empty predicates remain hard errors and surface as arrow::Status::Invalid. Unsupported expression forms (function calls, NULL literals, ...) and comparison type mismatches now cause only the affected sub-expression to be dropped: an offending AND conjunct is removed and the surviving conjuncts pushed down; an OR or NOT with a dropped child degrades the whole filter to no-filter (safe over-approximation). Each drop is logged to stderr inside the Rust parser. See milvus-io#512 Signed-off-by: Ted Xu <ted.xu@zilliz.com>
33c64f8 to
30e2d00
Compare
|
/lgtm |
3 participants
See #512
Implement predicate pushdown for Vortex files using sqlparser-rs
to parse SQL WHERE-clause predicates into native Vortex expressions.
The predicate string from get_record_batch_reader(predicate) is
parsed once in Rust, converted to a vortex::expr::Expression, and
applied via ScanBuilder::WithFilter() for zone-map pruning and
row-level filtering. No expression tree crosses the FFI boundary.
Changes:
IN lists, quoted column names for all-digit fields)
ScanBuilder integration
-> ColumnGroupReader -> FormatReader::set_predicate()
in read() via WithFilter()
filesystem I/O metrics
Benchmark highlights (40K rows, Vortex, local SSD):