alamb commented on code in PR #16930:
URL: https://github.com/apache/datafusion/pull/16930#discussion_r2240750126


##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -375,7 +375,19 @@ impl PhysicalExpr for BinaryExpr {
                 // as it takes into account cases where the selection contains 
null values.
                 let batch = filter_record_batch(batch, selection)?;
                 let right_ret = self.right.evaluate(&batch)?;
-                return pre_selection_scatter(selection, right_ret);
+                match &right_ret {
+                    ColumnarValue::Array(array) => {
+                        return pre_selection_scatter(selection, 
array.as_boolean());
+                    }
+                    ColumnarValue::Scalar(scalar) => {
+                        if let ScalarValue::Boolean(v) = scalar {

Review Comment:
   You can probably do better here than expanding out the entire array -- 
   
   specifically if the right value is `ScalarValue::Boolean(false)` we know 
nothing matches so can't we return a single 
ColumnValue(ScalarValue::Boolean(false))`
   
   Likewise if we know it is `ScalarValue::Boolean(true)` we can avoid 
selection gathering at all (and just pass back the selection)
   



##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -375,7 +375,19 @@ impl PhysicalExpr for BinaryExpr {
                 // as it takes into account cases where the selection contains 
null values.
                 let batch = filter_record_batch(batch, selection)?;
                 let right_ret = self.right.evaluate(&batch)?;
-                return pre_selection_scatter(selection, right_ret);
+                match &right_ret {
+                    ColumnarValue::Array(array) => {
+                        return pre_selection_scatter(selection, 
array.as_boolean());
+                    }
+                    ColumnarValue::Scalar(scalar) => {
+                        if let ScalarValue::Boolean(v) = scalar {
+                            let array = BooleanArray::from(vec![*v; 
batch.num_rows()]);
+                            return pre_selection_scatter(selection, &array);
+                        } else {
+                            return Ok(right_ret);

Review Comment:
   What other kind of scalar value could there be here? Would it be better to 
return an internal error if we get some non boolean value
   
   



-- 
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]

Reply via email to