[rust] Add predicates as part of filter pushdown - #3847
Conversation
f82f8c4 to
4ca085d
Compare
|
@charlesdong1991 @fresh-borzoni Appreciate a review here please 🙏 |
fresh-borzoni
left a comment
fresh-borzoni
left a comment
There was a problem hiding this comment.
@leekeiabstraction Thank you, left one question, PTAL
| //! let p = col("age").gt(30i64).and(col("name").starts_with("A")); | ||
| //! ``` | ||
| //! | ||
| //! The server prunes whole Arrow batches by their statistics, so a filtered scan |
There was a problem hiding this comment.
On the Java side, TableScan throws when the log format is not Arrow, and throws again if a filter is set on a batch scanner. The same code written against this API would silently do nothing instead, which is a hard thing to discover.
Should we mirror those checks when the predicate is attached to a scan?
There was a problem hiding this comment.
Agreed, I've added an additional doc line
Setting a predicate on a scan is only supported on log scans over tables with the ARROW log format.
The behaviour to reject for batch scan or non-arrow log will be implemented in PR for #3844
|
|
||
| #[test] | ||
| fn wire_codes_match_the_protocol() { | ||
| assert_eq!(LeafFunction::Equal.code(), 0); |
There was a problem hiding this comment.
why do we test only 5 out if 13?
There was a problem hiding this comment.
Nice catch, addressed.
Add the client-side predicate tree and `col()` builder that server-side filter pushdown will encode onto the FetchLog request. Column names and literals stay unresolved until the predicate is bound to a scan schema.
4ca085d to
4475ac2
Compare
|
@fresh-borzoni TY for the review. Addressed your comments, PTAL 🙏 |
charlesdong1991
left a comment
charlesdong1991
left a comment
There was a problem hiding this comment.
looks good to me!
| } | ||
|
|
||
| /// `column IN (values...)`. An empty `values` matches nothing. | ||
| pub fn is_in<V: Into<Literal>>(self, values: impl IntoIterator<Item = V>) -> Predicate { |
There was a problem hiding this comment.
when i check PredicateBuilder, it emits a real in leaf when >20 literals, otherwise or on equals due to performance reason, maybe nice to add a note not to revisit later
| } | ||
|
|
||
| #[test] | ||
| fn builds_leaf_predicates() { |
There was a problem hiding this comment.
we likely have no need to test all functions since it's quite straightforward, but fyi here
fresh-borzoni
left a comment
fresh-borzoni
left a comment
There was a problem hiding this comment.
@leekeiabstraction LGTM, thank you
3 participants
Summary
fluss::predicate: an unresolved predicate tree (Predicate::Leaf/Compound) and acol()builder, the first piece of server-side filter pushdown for Rust log scans.and/orflatten into n-ary nodes to match the wire representation. The protocol has no negation node, so negation is expressed withne/not_in.Test Plan
cargo test -p fluss-rs --lib— 592 passed, including 6 new tests covering leaf construction, literal conversion, combinator flattening and the protocol wire codes.cargo fmt/cargo clippy -p fluss-rs --libclean, apart from expecteddead_codeon the wire-code helpers the encoder will consume.🤖 AI-assisted changes - reviewed by human developer