-
Notifications
You must be signed in to change notification settings - Fork 28.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-45039][SQL] Include full identifier in Storage tab #42759
[SPARK-45039][SQL] Include full identifier in Storage tab #42759
Conversation
This reverts commit c6f0987.
…ure/improve_cache_storage_view
CC @cloud-fan |
The error in the test does not appear to be related to change |
spark.sessionState.sqlParser.parseMultipartIdentifier(table) match { | ||
case Seq(_) if spark.catalog.getTable(table).isTemporary => table | ||
|
||
case CatalogAndIdentifier(cat, ident) => s"${cat.name()}.${ident.toString}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should ask the caller side to pass in a fully qualified table name, instead of relying on the current catalog/namespace here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, I will make the changes, thank you!
@cloud-fan I made the changes, what do you think? thanks! |
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
What changes were proposed in this pull request?
If you have 2 databases with tables with the same name like: db1.table, db2.table
You cannot differentiate it in the Storage tab
What is proposed is to add the full identifier
With this PR it’s also modified the node name in the sql tab
Why are the changes needed?
Facilitates the interpretation of the Storage tab
Does this PR introduce any user-facing change?
Yes, in the UI
How was this patch tested?
Unit testing and manual tests
Was this patch authored or co-authored using generative AI tooling?
No