alamb commented on code in PR #17706:
URL: https://github.com/apache/datafusion/pull/17706#discussion_r2367482521
##########
datafusion/common/src/dfschema.rs:
##########
@@ -1129,7 +1129,9 @@ impl TryFrom<SchemaRef> for DFSchema {
field_qualifiers: vec![None; field_count],
functional_dependencies: FunctionalDependencies::empty(),
};
- dfschema.check_names()?;
+ // Without checking names, because schema here may have duplicate
field names.
+ // For example, Partial AggregateMode will generate duplicate field
names from
+ // state_fields.
Review Comment:
I double checked, and this is a new check that was added in 50.0.0 (via the
PR from @findepi @zhuqi-lucas mentions)
- https://github.com/apache/datafusion/pull/17189
Thus I think it is ok to disable this check for now (go back to the DF 49
behavior) and file a follow on ticket to validate the names from
`state_fields()`
##########
datafusion/common/src/dfschema.rs:
##########
@@ -1129,7 +1129,9 @@ impl TryFrom<SchemaRef> for DFSchema {
field_qualifiers: vec![None; field_count],
functional_dependencies: FunctionalDependencies::empty(),
};
- dfschema.check_names()?;
+ // Without checking names, because schema here may have duplicate
field names.
+ // For example, Partial AggregateMode will generate duplicate field
names from
+ // state_fields.
Review Comment:
```suggestion
// For example, Partial AggregateMode will generate duplicate field
names from
// state_fields.
// See <TODO FILE TICKET>
// dfschema.check_names()?;
```
--
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]
