paleolimbot commented on code in PR #835:
URL: https://github.com/apache/sedona-db/pull/835#discussion_r3239196292
##########
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:
Optional but possibly cleaner:
```suggestion
if let Some(combined) = conjunction(exprs.into_iter().map(|e|
e.inner)) {
let inner = self.inner.clone().filter(combined)?;
Ok(InternalDataFrame::new(inner, self.runtime.clone()))
} else {
Err(PySedonaError::SedonaPython(
"filter() requires at least one predicate".to_string(),
));
}
```
--
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]
