adriangb commented on code in PR #20694:
URL: https://github.com/apache/datafusion/pull/20694#discussion_r2886382069
##########
datafusion/physical-expr/src/expressions/in_list.rs:
##########
@@ -793,63 +793,71 @@ impl PhysicalExpr for InListExpr {
// comparator for unsupported types (nested, RunEndEncoded,
etc.).
let value = value.into_array(num_rows)?;
let lhs_supports_arrow_eq =
supports_arrow_eq(value.data_type());
- let found = self.list.iter().map(|expr|
expr.evaluate(batch)).try_fold(
- BooleanArray::new(BooleanBuffer::new_unset(num_rows),
None),
- |result, expr| -> Result<BooleanArray> {
- let rhs = match expr? {
- ColumnarValue::Array(array) => {
- if lhs_supports_arrow_eq
- && supports_arrow_eq(array.data_type())
- {
- arrow_eq(&value, &array)?
- } else {
- let cmp = make_comparator(
- value.as_ref(),
- array.as_ref(),
- SortOptions::default(),
- )?;
- (0..num_rows)
- .map(|i| {
- if value.is_null(i) ||
array.is_null(i) {
- return None;
- }
- Some(cmp(i, i).is_eq())
- })
- .collect::<BooleanArray>()
- }
+
+ // Helper: compare value against a single list expression
+ let compare_one = |expr: &Arc<dyn PhysicalExpr>| ->
Result<BooleanArray> {
+ match expr.evaluate(batch)? {
+ ColumnarValue::Array(array) => {
+ if lhs_supports_arrow_eq
+ && supports_arrow_eq(array.data_type())
+ {
+ Ok(arrow_eq(&value, &array)?)
+ } else {
+ let cmp = make_comparator(
+ value.as_ref(),
+ array.as_ref(),
+ SortOptions::default(),
+ )?;
+ let buffer =
BooleanBuffer::collect_bool(num_rows, |i| {
+ cmp(i, i).is_eq()
+ });
+ let nulls =
+ NullBuffer::union(value.nulls(),
array.nulls());
+ Ok(BooleanArray::new(buffer, nulls))
}
- ColumnarValue::Scalar(scalar) => {
- // Check if scalar is null once, before the
loop
- if scalar.is_null() {
- // If scalar is null, all comparisons
return null
- BooleanArray::from(vec![None; num_rows])
- } else if lhs_supports_arrow_eq {
- let scalar_datum = scalar.to_scalar()?;
- arrow_eq(&value, &scalar_datum)?
- } else {
- // Convert scalar to 1-element array
- let array = scalar.to_array()?;
- let cmp = make_comparator(
- value.as_ref(),
- array.as_ref(),
- SortOptions::default(),
- )?;
- // Compare each row of value with the
single scalar element
- (0..num_rows)
- .map(|i| {
- if value.is_null(i) {
- None
- } else {
- Some(cmp(i, 0).is_eq())
- }
- })
- .collect::<BooleanArray>()
- }
+ }
+ ColumnarValue::Scalar(scalar) => {
+ // Check if scalar is null once, before the loop
+ if scalar.is_null() {
+ // If scalar is null, all comparisons return
null
+ Ok(BooleanArray::from(vec![None; num_rows]))
+ } else if lhs_supports_arrow_eq {
+ let scalar_datum = scalar.to_scalar()?;
+ Ok(arrow_eq(&value, &scalar_datum)?)
+ } else {
+ // Convert scalar to 1-element array
+ let array = scalar.to_array()?;
+ let cmp = make_comparator(
+ value.as_ref(),
+ array.as_ref(),
+ SortOptions::default(),
+ )?;
+ // Compare each row of value with the single
scalar element
+ let buffer =
BooleanBuffer::collect_bool(num_rows, |i| {
+ cmp(i, 0).is_eq()
+ });
+ Ok(BooleanArray::new(buffer,
value.nulls().cloned()))
}
- };
- Ok(or_kleene(&result, &rhs)?)
- },
- )?;
+ }
+ }
+ };
+
+ // Evaluate first expression directly to avoid a redundant
+ // or_kleene with an all-false accumulator.
+ let mut found = if let Some(first) = self.list.first() {
+ compare_one(first)?
+ } else {
+ BooleanArray::new(BooleanBuffer::new_unset(num_rows), None)
+ };
+
+ for expr in self.list.iter().skip(1) {
+ // Short-circuit: if every non-null row is already true,
+ // no further list items can change the result.
+ if found.true_count() == num_rows {
Review Comment:
```suggestion
if found.null_count() == 0 && found.true_count() ==
num_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]
