haochunchang commented on code in PR #26202:
URL: https://github.com/apache/datafusion/pull/26202#discussion_r4388895429
##########
datafusion/sql/src/relation/mod.rs:
##########
@@ -90,7 +91,124 @@ impl<S: ContextProvider> RelationPlannerContext for
SqlToRelRelationContext<'_,
}
}
+/// A relation name used more than once in a `FROM` scope.
+pub(crate) struct DuplicateRelationName {
+ qualifier: TableReference,
+ /// Span of the first relation with this name, if known.
+ first_span: Option<Span>,
+ /// Span of the repeated relation.
+ span: Option<Span>,
+}
+
+impl DuplicateRelationName {
+ fn diagnostic(&self, new: fn(String, Option<Span>) -> Diagnostic) ->
Diagnostic {
+ let msg = format!("table name \"{}\" specified more than once",
self.qualifier);
+ let diagnostic = new(msg, self.span);
+ match self.first_span {
+ Some(first) => diagnostic.with_note("first defined here",
Some(first)),
+ None => diagnostic,
+ }
+ }
+}
+
impl<S: ContextProvider> SqlToRel<'_, S> {
+ /// Finds relations in the same `FROM` scope that are visible under the
+ /// same name. This never fails planning: callers report each result as a
+ /// warning and, if planning then fails on the duplicate qualifier, attach
+ /// it to the error (see [`Self::with_duplicate_diagnostic`]).
+ pub(crate) fn find_duplicate_relation_names<'a>(
+ &self,
+ relations: impl IntoIterator<Item = &'a TableFactor>,
+ ) -> Vec<DuplicateRelationName> {
+ let mut seen: HashMap<TableReference, Option<Span>> = HashMap::new();
+ let mut duplicates = vec![];
+ for relation in relations {
+ let Some((qualifier, span)) = self.extract_relation_name(relation)
else {
+ continue;
+ };
+ if let Some(first_span) = seen.get(&qualifier) {
+ duplicates.push(DuplicateRelationName {
+ qualifier,
+ first_span: *first_span,
+ span,
+ });
+ } else {
+ seen.insert(qualifier, span);
+ }
+ }
+ duplicates
+ }
+
+ /// Adds one warning per duplicate relation name.
+ pub(crate) fn warn_duplicate_relation_names(
+ &self,
+ duplicates: &[DuplicateRelationName],
+ ) {
+ for duplicate in duplicates {
+ self.add_warning(duplicate.diagnostic(Diagnostic::new_warning));
+ }
+ }
+
+ /// If `err` is a duplicate qualified field error for one of `duplicates`,
+ /// attaches a [`Diagnostic`] pointing at both relations. The error message
+ /// is unchanged.
+ pub(crate) fn with_duplicate_diagnostic(
+ err: DataFusionError,
+ duplicates: &[DuplicateRelationName],
+ ) -> DataFusionError {
+ let diagnostic = match err.find_root() {
+ DataFusionError::SchemaError(schema_err, _) => match &**schema_err
{
+ SchemaError::DuplicateQualifiedField { qualifier, .. } =>
duplicates
+ .iter()
+ .find(|d| d.qualifier == **qualifier)
+ .map(|d| d.diagnostic(Diagnostic::new_error)),
+ _ => None,
+ },
+ _ => None,
+ };
+ match diagnostic {
+ Some(diagnostic) => err.with_diagnostic(diagnostic),
+ None => err,
+ }
+ }
+
+ /// Returns the qualified name a relation is visible under in its `FROM`
+ /// scope (matching the qualifier of the planned relation), and the span of
+ /// that name. Returns `None` when the name cannot be resolved here, so
+ /// that planning reports the real error.
+ fn extract_relation_name(
+ &self,
+ relation: &TableFactor,
+ ) -> Option<(TableReference, Option<Span>)> {
+ match relation {
+ TableFactor::Table { alias: Some(a), .. }
+ | TableFactor::Derived { alias: Some(a), .. }
+ | TableFactor::Function { alias: Some(a), .. }
+ | TableFactor::UNNEST { alias: Some(a), .. }
+ | TableFactor::NestedJoin { alias: Some(a), .. } => {
Review Comment:
I added `comma_unaliased_nested_join` and
`explicit_join_unaliased_nested_join` cases inside
`test_duplicate_table_name` in `diagnostic.rs` to cover the unaliased nested
join case and its reverse form.
Before the fix: got expected 1 warning in comma_unaliased_nested_join
Also added 2 guard tests to prevent fixing too far.
I added `visible_relations` to traverse recursively on the unaliased nested
joins inside the relations.
And inside the TableFactor::NestedJoin arm of `create_default_relation`
function to stop duplicate checks when alias is None.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
