kosiew commented on code in PR #25469:
URL: https://github.com/apache/datafusion/pull/25469#discussion_r4310453061
##########
datafusion/physical-expr/src/utils/guarantee.rs:
##########
@@ -377,6 +388,85 @@ impl<'a> GuaranteeBuilder<'a> {
}
}
+/// Project necessary per-column guarantees; the original predicate retains
tuple correlation.
+fn project_struct_in_list(
+ inlist: &crate::expressions::InListExpr,
+) -> Option<Vec<(&crate::expressions::Column, Vec<ScalarValue>)>> {
+ if inlist.negated() || inlist.is_empty() {
+ return None;
+ }
+ let expr = inlist.expr().downcast_ref::<crate::ScalarFunctionExpr>()?;
+ let literal_args = expr
+ .args()
+ .iter()
+ .map(|arg| {
+ arg.downcast_ref::<crate::expressions::Literal>()
+ .map(|lit| lit.value().clone())
+ })
+ .collect::<Vec<_>>();
+ let mapping = expr.fun().struct_field_mapping(&literal_args)?;
+ if mapping.field_accessor.signature().volatility != Volatility::Immutable {
+ return None;
+ }
+ let tuples = inlist
+ .list()
+ .iter()
+ .map(|value| {
+ let literal = value.downcast_ref::<crate::expressions::Literal>()?;
+ let ScalarValue::Struct(array) = literal.value() else {
+ return None;
+ };
+ (array.len() == 1).then_some(literal.value())
+ })
+ .collect::<Option<Vec<_>>>()?;
+ // Null tuples cannot make a positive IN predicate true.
+ let tuples = ScalarValue::iter_to_array(
Review Comment:
Could we add a test where all RHS tuples are NULL, using `Some(vec![false,
false, false].into())`, and verify that no guarantees are extracted? The
current fallback is safe, but this would help ensure it stays that way.
##########
datafusion/physical-expr/src/utils/guarantee.rs:
##########
@@ -377,6 +388,85 @@ impl<'a> GuaranteeBuilder<'a> {
}
}
+/// Project necessary per-column guarantees; the original predicate retains
tuple correlation.
+fn project_struct_in_list(
+ inlist: &crate::expressions::InListExpr,
+) -> Option<Vec<(&crate::expressions::Column, Vec<ScalarValue>)>> {
+ if inlist.negated() || inlist.is_empty() {
+ return None;
+ }
+ let expr = inlist.expr().downcast_ref::<crate::ScalarFunctionExpr>()?;
+ let literal_args = expr
+ .args()
+ .iter()
+ .map(|arg| {
+ arg.downcast_ref::<crate::expressions::Literal>()
+ .map(|lit| lit.value().clone())
+ })
+ .collect::<Vec<_>>();
+ let mapping = expr.fun().struct_field_mapping(&literal_args)?;
+ if mapping.field_accessor.signature().volatility != Volatility::Immutable {
+ return None;
+ }
+ let tuples = inlist
+ .list()
+ .iter()
+ .map(|value| {
+ let literal = value.downcast_ref::<crate::expressions::Literal>()?;
+ let ScalarValue::Struct(array) = literal.value() else {
+ return None;
+ };
+ (array.len() == 1).then_some(literal.value())
+ })
+ .collect::<Option<Vec<_>>>()?;
+ // Null tuples cannot make a positive IN predicate true.
+ let tuples = ScalarValue::iter_to_array(
+ tuples.into_iter().filter(|tuple| !tuple.is_null()).cloned(),
+ )
+ .ok()?;
+ let batch = RecordBatch::try_from_iter([("tuple", tuples)]).ok()?;
+
+ let mut projected = Vec::new();
+ for (accessor_args, source_index) in mapping.fields {
+ let column = expr
+ .args()
+ .get(source_index)?
+ .downcast_ref::<crate::expressions::Column>()?;
Review Comment:
As a possible follow-up, could we skip computed arguments instead of
abandoning all projections? For `struct(a, b + 1) IN ((1, 10), (2, 20))`, we
could still extract `a IN (1, 2)`. Invalid mapping indices should still abort
extraction, and tests could cover computed arguments before and after a direct
column. No change is needed for this PR.
--
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]
