haochunchang commented on code in PR #26202:
URL: https://github.com/apache/datafusion/pull/26202#discussion_r4388937639
##########
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 {
Review Comment:
Yes, it intentionally preserves existing behavior.
I will update the PR description to match it and confirm them by auditing
the new tests and not changing/breaking existing tests.
--
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]
