aalexandrov commented on code in PR #20990:
URL: https://github.com/apache/datafusion/pull/20990#discussion_r3015407690
##########
datafusion/expr/src/utils.rs:
##########
Review Comment:
I don't have a strong preference here—in the initial PR I was trying to do
the minimal amount of changes to the existing method definition to fix the
error.
I did some cosmetic changes to your suggestion and added it to the PR. I
went with this `for`-loop body
```rust
if seen_names.contains(col.name.as_str()) {
excluded.insert(col); // exclude columns with already seen name
} else {
seen_names.insert(col.name.clone()); // mark column name as seen
}
```
because `seen_names.contains(col.name.clone())` will create unnecessary
`String` clones for ~50% of the `cols` entries.
--
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]
