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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
