feat: Attach Diagnostic to duplicate table name error#21395
Open
amitvijapur wants to merge 1 commit intoapache:mainfrom
Open
feat: Attach Diagnostic to duplicate table name error#21395amitvijapur wants to merge 1 commit intoapache:mainfrom
amitvijapur wants to merge 1 commit intoapache:mainfrom
Conversation
Attach a `Diagnostic` with span information to the "WITH query name specified more than once" error, so that tools can highlight the duplicate CTE name and point back to the first definition. Closes apache#14436 (CTE case; table alias case deferred — error originates in DFSchema::check_names() without SQL AST span access). Constraint: match the Diagnostic pattern from PR apache#15209 and existing codebase usage of plan_err!(...; diagnostic = diagnostic) Rejected: table alias duplicate in same PR | error site in DFSchema has no AST spans, requires deeper refactor inappropriate for a first PR Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Hi @amitvijapur, thanks for working on this. Just a heads up, I have an open PR #20720 from a month ago that also covers this issue, including duplicate CTE names as well as duplicate table aliases in FROM clauses. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Diagnosticto "duplicate table name" error #14436 (partial — covers duplicate CTE name diagnostic)Rationale for this change
This PR follows the pattern established in PR #15209 to attach
Diagnosticmetadata to SQL planning errors. Specifically, it adds span-based diagnostics to the "duplicate CTE name" error so that users get precise source locations when a CTE name is reused.What changes are included in this PR?
datafusion/sql/src/cte.rsto attach aDiagnosticwith a span pointing to the duplicate CTE name when aSchemaError::DuplicateQualifiedFieldis raised.datafusion/sql/tests/cases/diagnostic.rsverifying the diagnostic is correctly attached.Are these changes tested?
Yes — a new test case is added in
diagnostic.rsthat parses a query with a duplicate CTE name and asserts the resulting error carries the expectedDiagnostic.Are there any user-facing changes?
No breaking changes. Users will now see richer error diagnostics (with source spans) when they accidentally reuse a CTE name.
🤖 Generated with Claude Code