pepijnve commented on code in PR #17973:
URL: https://github.com/apache/datafusion/pull/17973#discussion_r2419160653
##########
datafusion/physical-expr-common/src/physical_expr.rs:
##########
@@ -97,14 +97,26 @@ pub trait PhysicalExpr: Any + Send + Sync + Display + Debug
+ DynEq + DynHash {
batch: &RecordBatch,
selection: &BooleanArray,
) -> Result<ColumnarValue> {
- let tmp_batch = filter_record_batch(batch, selection)?;
+ let selection_count = selection.true_count();
- let tmp_result = self.evaluate(&tmp_batch)?;
+ if batch.num_rows() == 0 || selection_count == batch.num_rows() {
Review Comment:
There was no assertion for this in place and as far as I can tell (but I did
not verify yet), you can call `evaluate_selection` today with a mismatch
between `batch.num_rows()` and `selection.len()` and it will do something. I
haven't tested this yet, so I'm not 100% sure what the outcome would be.
I'll write an extra unit test to cover this.
The idea here is to avoid any extra work whatsoever in the trivial cases.
Not sure what a useful, non-trivial comment for that would be.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
