rgbuilds commented on code in PR #25469:
URL: https://github.com/apache/datafusion/pull/25469#discussion_r4110344145
##########
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 {
Review Comment:
I think duplicate `named_struct` field names can make this inference unsound.
`named_struct` currently permits duplicate names and emits a mapping entry
for each field, while `get_field` uses `StructArray::column_by_name`, which
selects the first matching field when names are duplicated.
For example:
```sql
named_struct('k', x, 'k', y)
IN (named_struct('k', 1, 'k', 10))
```
Both mapping entries evaluate `get_field(..., 'k')` against the first
literal field. This appears to derive:
```text
x IN (1)
y IN (1)
```
However, `(x=1, y=10)` satisfies the original tuple predicate.
A row group containing the matching row `(1,10)` and a filler row `(0,0)`
would have `y_min=0` and `y_max=10`, so statistics cannot eliminate `y=1`. Its
Bloom filter could nevertheless prove that `y=1` is absent and incorrectly
prune the row group containing the valid `(1,10)` match.
Would it be safer for `named_struct::struct_field_mapping` to return no
mapping when an accessor name is repeated, with a focused regression test for
this case?
--
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]
