You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/01/16 12:44:32 UTC
[GitHub] [spark] MaxGekk opened a new pull request #31206: [WIP][SQL] Keep dependants cached while refreshing v1 tables
MaxGekk opened a new pull request #31206:
URL: https://github.com/apache/spark/pull/31206
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764533491
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38910/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763566738
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38856/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [WIP][SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761558979
**[Test build #134139 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134139/testReport)** for PR 31206 at commit [`0310c35`](https://github.com/apache/spark/commit/0310c35bd266cd87e99c7b07924cff9498dc5724).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560660708
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableAddPartitionSuite.scala
##########
@@ -123,6 +123,40 @@ trait AlterTableAddPartitionSuiteBase extends command.AlterTableAddPartitionSuit
}
}
}
+
+ test("SPARK-34138: keep dependents cashed after table altering") {
Review comment:
ditto. `cashed` -> `cached`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763684123
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134268/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763637301
**[Test build #134268 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134268/testReport)** for PR 31206 at commit [`f5302c6`](https://github.com/apache/spark/commit/f5302c6ca2fd600bb5fbdba67daa395cfb96735e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561612958
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
Seems it's not the case before according to https://github.com/apache/spark/pull/31206/files#r561316669 , @MaxGekk can you fix it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561612958
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
Seems it's not the case before according to https://github.com/apache/spark/pull/31206/files#r561316669 , @MaxGekk can you fix it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763591544
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38856/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763950203
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [WIP][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761575287
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38722/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761699364
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134145/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764139409
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134291/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560629702
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
Review comment:
This seems duplicated with the above statement `re-cache the table and its dependents lazily.`. Maybe we should change the above to `re-cache the table lazily.`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763527249
**[Test build #134268 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134268/testReport)** for PR 31206 at commit [`f5302c6`](https://github.com/apache/spark/commit/f5302c6ca2fd600bb5fbdba67daa395cfb96735e).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560742660
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandTestUtils.scala
##########
@@ -108,4 +108,15 @@ trait DDLCommandTestUtils extends SQLTestUtils {
}
size
}
+
+ def cacheRelation(name: String): Unit = {
+ assert(!spark.catalog.isCached(name))
Review comment:
To be sure that `CACHE TABLE` really works. If a table was already cached, the second assert after `sql(s"CACHE TABLE $name")` is useless.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560466771
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
Review comment:
Here, we are refreshing what is resolved first from the name passed to the function. The changes preserve existing behavior (if not, some tests should fail ... or we don't have any tests for the situation you describing here?).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560660309
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableRenamePartitionSuiteBase.scala
##########
@@ -178,4 +178,36 @@ trait AlterTableRenamePartitionSuiteBase extends QueryTest with DDLCommandTestUt
QueryTest.checkAnswer(sql(s"SELECT * FROM $t"), Seq(Row(0, 2), Row(1, 1)))
}
}
+
+ test("SPARK-34138: keep dependents cashed after table altering") {
Review comment:
ditto. `cashed` -> `cached`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31206:
URL: https://github.com/apache/spark/pull/31206
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560781685
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
Review comment:
why not follow `def invalidateCachedTable(key: QualifiedTableName): Unit`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560544203
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
Review comment:
I think previously we'd only call `invalidateCachedTable` when the table is not a view but now we'll always call that. Yes I feel we don't have complete test coverage for this piece of code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763902909
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560741330
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala
##########
@@ -181,4 +181,38 @@ trait AlterTableDropPartitionSuiteBase extends QueryTest with DDLCommandTestUtil
checkPartitions(t)
}
}
+
+ test("SPARK-34138: keep dependents cashed after table altering") {
Review comment:
IMHO, it has slightly different meaning. @srowen WDYT, how to say "table dependents are still cached after table altering". Maybe just say exactly like this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763487870
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38847/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764558455
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38910/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao edited a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
sunchao edited a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761615861
Thanks for pinging me. I'll take a look later today or tomorrow.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761644393
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38728/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761615861
Thanks for pinging me. I'll take a look later today.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763451786
**[Test build #134261 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134261/testReport)** for PR 31206 at commit [`269aefe`](https://github.com/apache/spark/commit/269aefe9242c2669c16d16c1b49799730b445e52).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763487870
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38847/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764628477
thanks, merging to master!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [WIP][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761575287
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38722/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763524278
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134261/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764492608
**[Test build #134323 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134323/testReport)** for PR 31206 at commit [`357c69f`](https://github.com/apache/spark/commit/357c69f888ed4f27ea8db3b78d79d123b4dac019).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763684123
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134268/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763479641
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38847/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560659737
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala
##########
@@ -181,4 +181,38 @@ trait AlterTableDropPartitionSuiteBase extends QueryTest with DDLCommandTestUtil
checkPartitions(t)
}
}
+
+ test("SPARK-34138: keep dependents cashed after table altering") {
Review comment:
`cashed` -> `cached`. 😉
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560468413
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
ok. I will add a comment. We refreshing the `child` because `relation` is an alias which has 2 fields - identifier and plan for the relation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763950203
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38877/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764497224
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38903/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560662395
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
Review comment:
nit. Shall we follow the comment style around this function? e.g. `This method discards ...`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561235123
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
Review comment:
I added "This method ..."
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
ok. I will wrap the invalidation by the check:
```scala
if (!sessionCatalog.isTemporaryTable(tableIdent)) {
sessionCatalog.invalidateCachedTable(tableIdent)
}
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
Here is a fix for `SessionCatalog.refreshTable()`: https://github.com/apache/spark/pull/31265
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560782980
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
it doesn't matter too much but still better to avoid. If it's already the case before, then we don't need to fix it here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764139409
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134291/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763524278
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134261/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [WIP][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761564580
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38722/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761615129
**[Test build #134145 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134145/testReport)** for PR 31206 at commit [`809b30c`](https://github.com/apache/spark/commit/809b30cc3969b4850d241a9b52afed99476a60fb).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763950203
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38877/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764637866
**[Test build #134323 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134323/testReport)** for PR 31206 at commit [`357c69f`](https://github.com/apache/spark/commit/357c69f888ed4f27ea8db3b78d79d123b4dac019).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761612508
@sunchao @viirya @dongjoon-hyun @cloud-fan @HyukjinKwon Could you review this PR, please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560769852
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
Review comment:
Don't think it is a duplicate. It describes the case when a table is not cached. See the PR https://github.com/apache/spark/pull/30187 in which the statement was added by @sunchao and committed by @dongjoon-hyun .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560734161
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
This will not cause any correctness issue, right?
ok. Look at `SessionCatalog.refreshTable`:
https://github.com/apache/spark/blob/157b72ac9fa0057d5fd6d7ed52a6c4b22ebd1dfc/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L990-L1006
if `name` is a temp view, the function could invalidate a record in `tableRelationCache` for a table. but nothing bad happens, `tableRelationCache` will be filled in next time.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560468413
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
ok. I will add a comment. We are refreshing the `child` because `relation` is an alias which has 2 fields - identifier and plan for the relation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560629299
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
Review comment:
`SessionCatalog` is already internal, we don't need "private[sql]" here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560801821
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
it's not very different from assert, but with better error message, let's do that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763902909
**[Test build #134291 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134291/testReport)** for PR 31206 at commit [`0dcd285`](https://github.com/apache/spark/commit/0dcd285ec7703ef6576f8eeb3ccc09e4992a4edd).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561649229
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
ok. I will wrap the invalidation by the check:
```scala
if (!sessionCatalog.isTemporaryTable(tableIdent)) {
sessionCatalog.invalidateCachedTable(tableIdent)
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r562175143
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
Here is a fix for `SessionCatalog.refreshTable()`: https://github.com/apache/spark/pull/31265
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763902909
**[Test build #134291 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134291/testReport)** for PR 31206 at commit [`0dcd285`](https://github.com/apache/spark/commit/0dcd285ec7703ef6576f8eeb3ccc09e4992a4edd).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763591544
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38856/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764497224
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38903/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764067301
**[Test build #134291 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134291/testReport)** for PR 31206 at commit [`0dcd285`](https://github.com/apache/spark/commit/0dcd285ec7703ef6576f8eeb3ccc09e4992a4edd).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560738009
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
I would avoid using any asserts in functions that can be called by users directly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763902909
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761699364
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134145/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [WIP][SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761611735
**[Test build #134139 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134139/testReport)** for PR 31206 at commit [`0310c35`](https://github.com/apache/spark/commit/0310c35bd266cd87e99c7b07924cff9498dc5724).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560403263
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
Review comment:
Hmm, what if we have both a view and a table with the same name under the default db? will this refresh the table even though the view should be resolved first?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
nit: might add a comment why we are recache the child here? it is not intuitive when first reading it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763520127
**[Test build #134261 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134261/testReport)** for PR 31206 at commit [`269aefe`](https://github.com/apache/spark/commit/269aefe9242c2669c16d16c1b49799730b445e52).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `public class CharVarcharCodegenUtils `
* ` new AnalysisException(s\"Can not load class '$className' when registering \" +`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764462307
**[Test build #134316 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134316/testReport)** for PR 31206 at commit [`330ee3f`](https://github.com/apache/spark/commit/330ee3f869ab4da22ff289c9c751f8e209ba1610).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764462307
**[Test build #134316 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134316/testReport)** for PR 31206 at commit [`330ee3f`](https://github.com/apache/spark/commit/330ee3f869ab4da22ff289c9c751f8e209ba1610).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561316669
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
Review comment:
Yes I meant temp view. I think it's better to keep the old behavior (i.e., not invalidate `tableRelationCache` if the input is a temp view), if there is no valid reason not to do so.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560788122
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
If the `relation` is not `SubqueryAlias`, then the behavior is undefined for `relation.children.foreach...`. We should avoid that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761693328
**[Test build #134145 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134145/testReport)** for PR 31206 at commit [`809b30c`](https://github.com/apache/spark/commit/809b30cc3969b4850d241a9b52afed99476a60fb).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763451786
**[Test build #134261 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134261/testReport)** for PR 31206 at commit [`269aefe`](https://github.com/apache/spark/commit/269aefe9242c2669c16d16c1b49799730b445e52).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763551204
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38856/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763475891
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38847/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764628477
thanks, merging to master!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764665513
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134323/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561316669
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
Review comment:
Yes I meant temp view. I think it's better to keep the old behavior (i.e., not invalidate `tableRelationCache` if the input is a temp view), if there is no valid reason not to do so.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763920758
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38877/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764558455
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38910/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761644393
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38728/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764630063
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134316/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761614558
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134139/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764492608
**[Test build #134323 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134323/testReport)** for PR 31206 at commit [`357c69f`](https://github.com/apache/spark/commit/357c69f888ed4f27ea8db3b78d79d123b4dac019).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763527249
**[Test build #134268 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134268/testReport)** for PR 31206 at commit [`f5302c6`](https://github.com/apache/spark/commit/f5302c6ca2fd600bb5fbdba67daa395cfb96735e).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560630367
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
Review comment:
> what if we have both a view and a table with the same name under the default db?
It's not possible, view and table share the same namespace. Temp view is another story though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764480748
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38903/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761614558
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134139/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560802715
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
Review comment:
if you can't see it in https://spark.apache.org/docs/latest/api/scala/org/apache/spark/index.html , it's internal
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [WIP][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761558979
**[Test build #134139 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134139/testReport)** for PR 31206 at commit [`0310c35`](https://github.com/apache/spark/commit/0310c35bd266cd87e99c7b07924cff9498dc5724).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764522849
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38910/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764497203
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38903/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560747107
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
Review comment:
Not sure. `SessionCatalog` has more method comments without any `This method ...`. It should obvious that method comment describes `This method ...`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560738885
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
Review comment:
I just followed existing methods there:
https://github.com/apache/spark/blob/157b72ac9fa0057d5fd6d7ed52a6c4b22ebd1dfc/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L1652
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31206:
URL: https://github.com/apache/spark/pull/31206
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763950203
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761615129
**[Test build #134145 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134145/testReport)** for PR 31206 at commit [`809b30c`](https://github.com/apache/spark/commit/809b30cc3969b4850d241a9b52afed99476a60fb).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764665513
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134323/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560659737
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala
##########
@@ -181,4 +181,38 @@ trait AlterTableDropPartitionSuiteBase extends QueryTest with DDLCommandTestUtil
checkPartitions(t)
}
}
+
+ test("SPARK-34138: keep dependents cashed after table altering") {
Review comment:
`cashed` -> `cached`. 😉
BTW, maybe, `keep cached dependents after table altering` sounds better?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r561235123
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
Review comment:
I added "This method ..."
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [WIP][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-761569774
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38722/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764630063
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134316/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-764611743
**[Test build #134316 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134316/testReport)** for PR 31206 at commit [`330ee3f`](https://github.com/apache/spark/commit/330ee3f869ab4da22ff289c9c751f8e209ba1610).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560660596
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandTestUtils.scala
##########
@@ -108,4 +108,15 @@ trait DDLCommandTestUtils extends SQLTestUtils {
}
size
}
+
+ def cacheRelation(name: String): Unit = {
+ assert(!spark.catalog.isCached(name))
Review comment:
Just a question. Why do we assert this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560787419
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
Review comment:
@cloud-fan In general, how do you figure out that some class is an internal or not?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560630871
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
can we make it clearer in the code?
```
assert(relation.isInstanceOf[SubqueryAlias])
recacheByPlan(relation.asInstanceOf[SubqueryAlias].child)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560631515
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -170,6 +170,13 @@ class SessionCatalog(
tableRelationCache.invalidate(key)
}
+ /** Discards any cached table relation plans for the given table identifier. */
+ private[sql] def invalidateCachedTable(name: TableIdentifier): Unit = {
+ val dbName = formatDatabaseName(name.database.getOrElse(currentDb))
+ val tableName = formatTableName(name.table)
+ invalidateCachedTable(QualifiedTableName(dbName, tableName))
Review comment:
if the `name` refers to a temp view, we shouldn't call `invalidateCachedTable` here which may accidentally invalid cache for a table with the same name as the temp view.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31206:
URL: https://github.com/apache/spark/pull/31206#issuecomment-763950128
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38877/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560776171
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
@cloud-fan IMHO, direct casting makes code less reliable. Initially, I was going to implement like what you proposed here but decided to not cast since there is alternative approach.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #31206: [SPARK-34138][SQL] Keep dependants cached while refreshing v1 tables
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31206:
URL: https://github.com/apache/spark/pull/31206#discussion_r560797368
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
##########
@@ -512,49 +512,27 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
* For Hive metastore table, the metadata is refreshed. For data source tables, the schema will
* not be inferred and refreshed.
*
- * If this table is cached as an InMemoryRelation, drop the original cached version and make the
- * new version cached lazily.
+ * If this table is cached as an InMemoryRelation, re-cache the table and its dependents lazily.
*
- * In addition, refreshing a table also invalidate all caches that have reference to the table
+ * In addition, refreshing a table also clear all caches that have reference to the table
* in a cascading manner. This is to prevent incorrect result from the otherwise staled caches.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
- val tableMetadata = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
- val table = sparkSession.table(tableIdent)
+ val relation = sparkSession.table(tableIdent).queryExecution.analyzed
- if (tableMetadata.tableType == CatalogTableType.VIEW) {
- // Temp or persistent views: refresh (or invalidate) any metadata/data cached
- // in the plan recursively.
- table.queryExecution.analyzed.refresh()
- } else {
- // Non-temp tables: refresh the metadata cache.
- sessionCatalog.refreshTable(tableIdent)
- }
-
- // If this table is cached as an InMemoryRelation, drop the original
- // cached version and make the new version cached lazily.
- val cache = sparkSession.sharedState.cacheManager.lookupCachedData(table)
-
- // uncache the logical plan.
- // note this is a no-op for the table itself if it's not cached, but will invalidate all
- // caches referencing this table.
- sparkSession.sharedState.cacheManager.uncacheQuery(table, cascade = true)
-
- if (cache.nonEmpty) {
- // save the cache name and cache level for recreation
- val cacheName = cache.get.cachedRepresentation.cacheBuilder.tableName
- val cacheLevel = cache.get.cachedRepresentation.cacheBuilder.storageLevel
-
- // creates a new logical plan since the old table refers to old relation which
- // should be refreshed
- val newTable = sparkSession.table(tableIdent)
+ relation.refresh()
+ sessionCatalog.invalidateCachedTable(tableIdent)
- // recache with the same name and cache level.
- sparkSession.sharedState.cacheManager.cacheQuery(newTable, cacheName, cacheLevel)
+ // Re-caches the logical plan of the relation.
+ // Note this is a no-op for the relation itself if it's not cached, but will clear all
+ // caches referencing this relation. If this relation is cached as an InMemoryRelation,
+ // this will clear the relation cache and caches of all its dependants.
+ relation.children.foreach { child =>
Review comment:
How about this:
```scala
relation match {
case SubqueryAlias(_, relationPlan) =>
sparkSession.sharedState.cacheManager.recacheByPlan(sparkSession, relationPlan)
case _ => throw new AnalysisException(
s"Unexpected type ${relation.getClass.getCanonicalName} of the relation $tableName")
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org