Skip to content

Commit

Permalink
Fix: check ambiguous column reference
Browse files Browse the repository at this point in the history
  • Loading branch information
HuSen8891 committed Sep 16, 2024
1 parent 6590ea3 commit 10d7171
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 4 deletions.
54 changes: 50 additions & 4 deletions datafusion/common/src/dfschema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,7 @@ impl DFSchema {
name: &str,
) -> Result<(Option<&TableReference>, &Field)> {
if let Some(qualifier) = qualifier {
let idx = self
.index_of_column_by_name(Some(qualifier), name)
.ok_or_else(|| field_not_found(Some(qualifier.clone()), name, self))?;
Ok((self.field_qualifiers[idx].as_ref(), self.field(idx)))
self.qualified_field_with_qualified_name(qualifier, name)
} else {
self.qualified_field_with_unqualified_name(name)
}
Expand Down Expand Up @@ -467,6 +464,36 @@ impl DFSchema {
.collect()
}

/// Find all fields that match the given name with qualifier and return them with their qualifier
pub fn qualified_fields_with_qualified_name(
&self,
qualifier: &TableReference,
name: &str,
) -> Vec<(Option<&TableReference>, &Field)> {
self.iter()
.filter(|(q, f)| match (qualifier, q) {
// field to lookup is qualified.
// current field is qualified and not shared between relations, compare both
// qualifier and name.
(q, Some(field_q)) => q.resolved_eq(field_q) && f.name() == name,
// field to lookup is qualified but current field is unqualified.
(qq, None) => {
// the original field may now be aliased with a name that matches the
// original qualified name
let column = Column::from_qualified_name(f.name());
match column {
Column {
relation: Some(r),
name: column_name,
} => &r == qq && column_name == name,
_ => false,
}
}
})
.map(|(qualifier, field)| (qualifier, field.as_ref()))
.collect()
}

/// Find all fields that match the given name and convert to column
pub fn columns_with_unqualified_name(&self, name: &str) -> Vec<Column> {
self.iter()
Expand Down Expand Up @@ -519,6 +546,25 @@ impl DFSchema {
}
}

/// Find the qualified field with the given qualified name
pub fn qualified_field_with_qualified_name(
&self,
qualifier: &TableReference,
name: &str,
) -> Result<(Option<&TableReference>, &Field)> {
let matches = self.qualified_fields_with_qualified_name(qualifier, name);
match matches.len() {
0 => Err(field_not_found(Some(qualifier.clone()), name, self)),
1 => Ok((matches[0].0, (matches[0].1))),
_ => _schema_err!(SchemaError::AmbiguousReference {
field: Column {
relation: None,
name: name.to_string(),
},
}),
}
}

/// Find the field with the given name
pub fn field_with_unqualified_name(&self, name: &str) -> Result<&Field> {
self.qualified_field_with_unqualified_name(name)
Expand Down
5 changes: 5 additions & 0 deletions datafusion/sql/src/expr/identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let s = &ids[0..ids.len()];
// safe unwrap as s can never be empty or exceed the bounds
let (relation, column_name) = form_identifier(s).unwrap();
// sanity check on column
schema.qualified_field_with_name(
relation.as_ref(),
column_name,
)?;
Ok(Expr::Column(Column::new(relation, column_name)))
}
}
Expand Down
14 changes: 14 additions & 0 deletions datafusion/sqllogictest/test_files/join.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1209,3 +1209,17 @@ drop table t1;

statement ok
drop table t2;

# Test SQLancer issue: https://github.com/apache/datafusion/issues/12337
statement ok
create table t1(v1 int);

## Query with Ambiguous column reference
query error DataFusion error: Schema error: Ambiguous reference to unqualified field v1
select count(*)
from t1
right outer join t1
on t1.v1 > 0;

statement ok
drop table t1;

0 comments on commit 10d7171

Please sign in to comment.