[SPARK-34347][SQL] CatalogImpl.uncacheTable should invalidate in cascade for temp views #31462
Conversation
sunchao
changed the title
|
Kubernetes integration test starting |
| Seq(true, false).foreach { storeAnalyzed => | ||
| withSQLConf(SQLConf.STORE_ANALYZED_PLAN_FOR_VIEW.key -> storeAnalyzed.toString) { | ||
| withTempView("view1", "view2") { | ||
| sql("CREATE TEMPORARY VIEW view1 AS SELECT * FROM testData WHERE key > 1") |
There was a problem hiding this comment.
How about GLOBAL TEMPORARY VIEW as well.
There was a problem hiding this comment.
Good point. Will add test for that.
|
Kubernetes integration test status failure |
|
Test build #134845 has finished for PR 31462 at commit
|
|
Kubernetes integration test starting |
| tempViews.get(tableName).map(getTempViewPlan) | ||
| } else if (formatDatabaseName(name.database.get) == globalTempViewManager.database) { | ||
| globalTempViewManager.get(tableName) | ||
| globalTempViewManager.get(tableName).map(getTempViewPlan) |
There was a problem hiding this comment.
Does this add a View wrapper which is not added previously?
There was a problem hiding this comment.
Yes. This is added so we can pass a parsed view plan to CatalogImpl.uncacheView. I think it also fixes a potential issue in refreshTable where refresh is called on TemporaryViewRelation instead of View.
|
Kubernetes integration test status success |
|
Test build #134858 has finished for PR 31462 at commit
|
|
Any more comments on this @viirya ? cc @dongjoon-hyun too. |
|
I'll merge this later if no more comments. |
HyukjinKwon
left a comment
HyukjinKwon
left a comment
There was a problem hiding this comment.
Looks making sense.
cc @linhongliu-db, @cloud-fan and @maryannxue FYI from e02324f and bac50aa
|
For clarification, it should be fine to merge it (and leave it for them to post-review). |
|
lgtm |
|
Thanks! Merging to master. |
7 participants
What changes were proposed in this pull request?
This PR includes the following changes:
CatalogImpl.uncacheTable, invalidate caches in cascade when the target table isa temp view, and
spark.sql.legacy.storeAnalyzedPlanForViewis false (default value).SessionCatalog.lookupTempViewpublic and return processed temp view plan (i.e., withViewop).Why are the changes needed?
Following SPARK-34052 (#31107), we should invalidate in cascade for
CatalogImpl.uncacheTablewhen the table is a temp view, so that the behavior is consistent.Does this PR introduce any user-facing change?
Yes, now
SQLContext.uncacheTablewill drop temp view in cascade by default.How was this patch tested?
Added a UT