alamb commented on code in PR #5328:
URL: https://github.com/apache/arrow-datafusion/pull/5328#discussion_r1111057588
##########
datafusion/common/src/dfschema.rs:
##########
@@ -314,6 +314,42 @@ impl DFSchema {
}
}
+ /// Find if the field exists with the given name
+ pub fn has_column_with_unqualified_name(&self, name: &str) -> Result<bool>
{
+ let matches = self.fields_with_unqualified_name(name);
+ match matches.len() {
+ 0 => Ok(false),
+ 1 => Ok(true),
+ _ => Err(DataFusionError::SchemaError(
+ SchemaError::AmbiguousReference {
+ qualifier: None,
+ name: name.to_string(),
Review Comment:
this `to_string()` is what kills performance (as it allocates a new string
for every ambiguous error)
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -351,7 +351,9 @@ impl LogicalPlanBuilder {
schema: _,
}) if missing_cols
.iter()
- .all(|c| input.schema().field_from_column(c).is_ok()) =>
+ // TODO: how to return err in closure, maybe there is hethod
like try_all in iter?
Review Comment:
the whole point if the `is_ok()` call is to ignore the error (aka the error
should not be returned)
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -656,13 +658,13 @@ impl LogicalPlanBuilder {
let mut join_on: Vec<(Expr, Expr)> = vec![];
let mut filters: Option<Expr> = None;
for (l, r) in &on {
- if self.plan.schema().field_from_column(l).is_ok()
- && right.schema().field_from_column(r).is_ok()
+ if self.plan.schema().has_column(l)?
Review Comment:
propagating the error via `?` is not correct (this isn't an error, it is
check, I think)
--
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]
