[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5328: [feat]:fast check has column

["alamb (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org