[GitHub] [arrow-datafusion] houqp commented on a change in pull request #810: Qualified field resolution too strict

[GitBox <gi...@apache.org>]

houqp commented on a change in pull request #810:
URL: https://github.com/apache/arrow-datafusion/pull/810#discussion_r680618351



##########
File path: datafusion/src/logical_plan/dfschema.rs
##########
@@ -160,7 +160,13 @@ impl DFSchema {
                 // field to lookup is qualified.
                 // current field is qualified and not shared between relations, compare both
                 // qualifer and name.
-                (Some(q), Some(field_q)) => q == field_q && field.name() == name,
+                (Some(q), Some(field_q)) => {

Review comment:
       nitpick, i think it would be good to check for `field.name() == name` first and do an early return if it doesn't match to avoid the subsequent field parsing/matching.

##########
File path: datafusion/src/sql/planner.rs
##########
@@ -925,11 +925,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
 
     /// Generate a relational expression from a SQL expression
     pub fn sql_to_rex(&self, sql: &SQLExpr, schema: &DFSchema) -> Result<Expr> {
-        let expr = self.sql_expr_to_logical_expr(sql, schema)?;
+        let mut expr = self.sql_expr_to_logical_expr(sql, schema)?;
         self.validate_schema_satisfies_exprs(schema, &[expr.clone()])?;
+        expr = self.rewrite_partial_qualifier(expr, schema);
         Ok(expr)
     }
 
+    /// Rewrite aliases which are not-complete (e.g. ones that only include only table qualifier in a schema.table qualified relation)
+    fn rewrite_partial_qualifier(&self, expr: Expr, schema: &DFSchema) -> Expr {
+        match expr.clone() {
+            Expr::Column(col) => match &col.relation {
+                Some(q) => {
+                    match schema
+                        .fields()
+                        .iter()
+                        .find(|field| match field.qualifier() {
+                            Some(field_q) if field_q.contains('.') => {
+                                field_q != q
+                                    && field_q.ends_with(q)

Review comment:
       same question here with regards to my comment on the change in `index_of_column_by_name`.

##########
File path: datafusion/src/logical_plan/dfschema.rs
##########
@@ -160,7 +160,13 @@ impl DFSchema {
                 // field to lookup is qualified.
                 // current field is qualified and not shared between relations, compare both
                 // qualifer and name.
-                (Some(q), Some(field_q)) => q == field_q && field.name() == name,
+                (Some(q), Some(field_q)) => {
+                    if field_q.contains('.') {
+                        field_q.ends_with(q) && field.name() == name

Review comment:
       Wouldn't this incorrectly return true when `q` is a strict suffix of the table name in `field_q`? It would be more robust to check whether `field_q` ends with `'.'` + `q` right? 




-- 
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