[SPARK-54873][SQL] Simplify V2TableReference resolution as only temp view may contain it #53646
Conversation
JIRA Issue Information=== Improvement SPARK-54873 === This comment was automatically generated by GitHub Actions |
| val nameInCache = v2Ident.toQualifiedNameParts(catalog) | ||
| isSameName(name, nameInCache, resolver) && (includeTimeTravel || timeTravelSpec.isEmpty) | ||
|
|
||
| case r: V2TableReference => |
There was a problem hiding this comment.
We only cache resolved plans, and we only perform cache lookup for resolved plans, so V2TableReference should never exist in the cache manager.
cloud-fan
changed the title
| V2TableReferenceUtils.validateLoadedTable(table, ref) | ||
| val tableName = ref.identifier.toQualifiedNameParts(ref.catalog) | ||
| SubqueryAlias(tableName, ref.toRelation(table)) | ||
| ref.toRelation(table) |
There was a problem hiding this comment.
V2TableReference was created from v2 relation, and should be restored back to v2 relation, without SubqueryAlias, so that it's a round trip.
There was a problem hiding this comment.
|
There is some upcoming work for transactional DML that may leverage V2 table reference beyond temp views. I am not 100% sure, however. |
| case u: UnresolvedRelation => | ||
| resolveRelation(u).map(resolveViews(_, u.options)).getOrElse(u) | ||
|
|
||
| case r: V2TableReference => |
There was a problem hiding this comment.
@aokolnychyi if I keep this resolution here, it should work for the future DML changes?
There was a problem hiding this comment.
I think so, I support the simplification but let's keep v2 reference generic for future use cases.
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
Show resolved
Hide resolved
…ysis/Analyzer.scala
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
Outdated
Show resolved
Hide resolved
…ysis/Analyzer.scala
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
Show resolved
Hide resolved
…ysis/Analyzer.scala
2 participants
What changes were proposed in this pull request?
Currently we resolve
V2TableReferenceat multiple places in the ruleResolveRelations. Actually only temp view with resolved plan (not SQL view) may containV2TableReference, and we only need to resolveV2TableReferencewhere we return the view plan.V2TableReferenceis marked as resolved, but it must be replaced withDataSourceV2Relationduring analysis. This PR also validates it inCheckAnalysis.Why are the changes needed?
Simplify code
Does this PR introduce any user-facing change?
No
How was this patch tested?
existing tests
Was this patch authored or co-authored using generative AI tooling?
cursor 2.2.44