jayzhan211 commented on code in PR #25892:
URL: https://github.com/apache/datafusion/pull/25892#discussion_r4306412212
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -5190,8 +5190,49 @@ impl Unnest {
let metadata = input_schema.metadata().clone();
let df_schema = DFSchema::new_with_metadata(fields, metadata)?;
- // We can use the existing functional dependencies:
- let deps = input_schema.functional_dependencies().clone();
+ let list_indices: HashSet<_> =
+ list_columns.iter().map(|(index, _)| *index).collect();
+ // Struct unnest may replace one input column with several output
columns,
+ // so remap dependencies using the input index of each output column.
+ let remap_indices = |indices: &[usize], exclude_lists: bool| {
+ dependency_indices
+ .iter()
+ .enumerate()
+ .filter_map(|(output_index, input_index)| {
+ (indices.contains(input_index)
+ && (!exclude_lists ||
!list_indices.contains(input_index)))
+ .then_some(output_index)
+ })
+ .collect()
+ };
+ let deps = FunctionalDependencies::new(
+ input_schema
+ .functional_dependencies()
+ .iter()
+ // A list input no longer determines the rows produced by
unnest.
+ // A struct input is also unsafe as a source: a null struct and
+ // a non-null struct with all-null fields have the same
outputs.
+ .filter(|dep| {
+ dep.source_indices.iter().all(|index| {
+ !list_indices.contains(index)
+ && !struct_columns.contains(index)
+ && dependency_indices.contains(index)
Review Comment:
nit: `dependency_indices.contains(index)` can't be false once the first two
checks pass. Every column that isn't unnested is pushed to
`dependency_indices`, and an unnested column is always in `list_indices` or
`struct_columns`, because `get_unnested_columns` errors on any other type
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -5190,8 +5190,49 @@ impl Unnest {
let metadata = input_schema.metadata().clone();
let df_schema = DFSchema::new_with_metadata(fields, metadata)?;
- // We can use the existing functional dependencies:
- let deps = input_schema.functional_dependencies().clone();
+ let list_indices: HashSet<_> =
+ list_columns.iter().map(|(index, _)| *index).collect();
+ // Struct unnest may replace one input column with several output
columns,
+ // so remap dependencies using the input index of each output column.
+ let remap_indices = |indices: &[usize], exclude_lists: bool| {
+ dependency_indices
+ .iter()
+ .enumerate()
+ .filter_map(|(output_index, input_index)| {
+ (indices.contains(input_index)
+ && (!exclude_lists ||
!list_indices.contains(input_index)))
+ .then_some(output_index)
+ })
+ .collect()
+ };
+ let deps = FunctionalDependencies::new(
+ input_schema
+ .functional_dependencies()
+ .iter()
+ // A list input no longer determines the rows produced by
unnest.
+ // A struct input is also unsafe as a source: a null struct and
+ // a non-null struct with all-null fields have the same
outputs.
+ .filter(|dep| {
+ dep.source_indices.iter().all(|index| {
+ !list_indices.contains(index)
+ && !struct_columns.contains(index)
+ && dependency_indices.contains(index)
Review Comment:
```suggestion
!list_indices.contains(index) &&
!struct_columns.contains(index)
```
--
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]
