jiayuasu commented on code in PR #835:
URL: https://github.com/apache/sedona-db/pull/835#discussion_r3239720426
##########
python/sedonadb/src/dataframe.rs:
##########
@@ -123,6 +123,32 @@ impl InternalDataFrame {
Ok(InternalDataFrame::new(inner, self.runtime.clone()))
}
+ /// Filter rows by one or more boolean expressions, producing a new
+ /// lazy `DataFrame`.
+ ///
+ /// The Python side guarantees `exprs` is non-empty and that every
+ /// element is an `Expr` (no strings, no Literals — those rejections
+ /// happen at the Python boundary so the error message can point at
+ /// the right alternative). Multiple predicates are combined into a
+ /// single composed `Expr` using DataFusion's `conjunction` helper,
+ /// which yields one conjunction node for the optimizer to reason
+ /// about rather than stacked filter nodes. Column-validity errors
+ /// surface at plan-build time from DataFusion, matching the
+ /// behavior of `select`.
+ fn filter(&self, exprs: Vec<PyExpr>) -> Result<InternalDataFrame,
PySedonaError> {
+ if exprs.is_empty() {
+ return Err(PySedonaError::SedonaPython(
+ "filter() requires at least one predicate".to_string(),
+ ));
+ }
+ // `conjunction` returns None only for an empty iterator, which we
+ // already rejected above; unwrap is safe.
+ let combined =
datafusion_expr::utils::conjunction(exprs.into_iter().map(|e| e.inner))
+ .expect("non-empty predicates checked above");
+ let inner = self.inner.clone().filter(combined)?;
+ Ok(InternalDataFrame::new(inner, self.runtime.clone()))
Review Comment:
Applied in 06fdfc7a — `filter()` now wraps the work in `if let
Some(combined) = conjunction(...)` with the empty-list branch in `else`,
dropping the `expect()`. (Removed the trailing `;` on the `Err(...)` line so
the else arm types as `Result`.)
--
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]
