rluvaton commented on code in PR #27302:
URL: https://github.com/apache/datafusion/pull/27302#discussion_r4525981628
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -1256,10 +1324,16 @@ enum ShortCircuitStrategy {
const PRE_SELECTION_THRESHOLD: f32 = 0.2;
/// Returns the short-circuit strategy for `OR` with a Boolean LHS.
-/// All true returns the LHS; all false returns the RHS. Sparse false rows
-/// trigger pre-selection at [`PRE_SELECTION_THRESHOLD`].
+/// All true returns the LHS; all false returns the RHS. Otherwise the RHS is
+/// evaluated only on the rows that are not `true` when at most
+/// [`PRE_SELECTION_THRESHOLD`] of the rows need it, or when the RHS is
volatile
+/// (it must never be evaluated on rows that don't need it).
/// `AND` uses [`BinaryExpr::evaluate_conjunction`].
-fn check_short_circuit(lhs: &ColumnarValue, op: &Operator) ->
ShortCircuitStrategy {
+fn check_short_circuit(
+ lhs: &ColumnarValue,
+ op: &Operator,
+ rhs_is_volatile: impl FnOnce() -> bool,
Review Comment:
commented in the other review comment
--
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]
