pepijnve commented on code in PR #17973:
URL: https://github.com/apache/datafusion/pull/17973#discussion_r2419550854
##########
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:
@alamb @findepi I've added some unit tests to help define the behaviour of
`evaluate_selection`. Some are currently still failing. Could you guys take a
look at the tests to see if what they're asserting is correct?
The issues are all related to record batch / selection vector size
mismatches. I don't think the current behaviour makes sense tbh (which is also
present on `main`). I would expect to either get an error or as many output
rows as there were input rows.
--
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]
