[SPARK-15367] [SQL] Add refreshTable back by gatorsmile · Pull Request #13156 · apache/spark

gatorsmile · 2016-05-17T22:06:49Z

What changes were proposed in this pull request?

refreshTable was a method in HiveContext. It was deleted accidentally while we were migrating the APIs. This PR is to add it back to HiveContext.

In addition, in SparkSession, we put it under the catalog namespace (SparkSession.catalog.refreshTable).

How was this patch tested?

Changed the existing test cases to use the function refreshTable. Also added a test case for refreshTable in hivecontext-compatibility

yhuai · 2016-05-17T22:08:26Z

sql/core/src/main/scala/org/apache/spark/sql/SQLContext.scala

+   */
+  def refreshTable(tableName: String): Unit = {
+    sparkSession.catalog.refreshTable(tableName)
+  }


This method did not exist in SQLContext. It's in HiveContext.

Sure, will remove it. Thanks!

SparkQA · 2016-05-17T23:40:50Z

Test build #58725 has finished for PR 13156 at commit 9e6c4b7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2016-05-18T04:21:21Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala

        .saveAsTable("arrayInParquet")

-      sessionState.refreshTable("arrayInParquet")
+      sparkSession.catalog.refreshTable("arrayInParquet")


As we don't call refreshTable through SessionState, do we still need to keep SessionState.refreshTable?

Actually, I also want to remove invalidateTable, which is a duplicate name of refreshTable

Actually, invalidateTable and refreshTable do have different meanings. The current implementation of HiveMetastoreCatalog.refreshTable is HiveMetastoreCatalog.invalidateTable (and then we retrieve the new metadata lazily). But, it does not mean that refreshTable and invalidateTable have the same semantic. If we should remove any of invalidateTable or refreshTable should be discussed in a different thread.

Got it. Thanks!

SparkQA · 2016-05-18T05:49:55Z

Test build #58740 has finished for PR 13156 at commit 4ac3b76.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2016-05-18T18:36:11Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala

-      sparkSession.cacheManager.tryUncacheQuery(df, blocking = true)
-      // Cache it again.
-      sparkSession.cacheManager.cacheQuery(df, Some(tableIdent.table))
-    }


The above logics are moved to sparkSession.catalog.refreshTable

SparkQA · 2016-05-18T20:34:31Z

Test build #58805 has finished for PR 13156 at commit 7142ef5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-05-18T20:54:11Z

Test build #58808 has finished for PR 13156 at commit 8a52ac6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2016-05-19T00:57:25Z

...t-compatibility/src/test/scala/org/apache/spark/sql/hive/HiveContextCompatibilitySuite.scala

+    val _hc = hc
+    import _hc.implicits._
+
+    withTempPath { tempDir =>


Do we still need this test?

Sure, let me remove it. : )

cloud-fan · 2016-05-19T00:59:39Z

LGTM except the test stuff, thanks for working on it! I agree that we should remove refreshTable in SessionState, but need someone to confirm, or we can do it in follow-up PR.

SparkQA · 2016-05-19T02:33:24Z

Test build #58831 has finished for PR 13156 at commit 2b773b8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class HiveContextCompatibilitySuite extends SparkFunSuite with BeforeAndAfterEach

cloud-fan · 2016-05-19T03:12:08Z

sql/hivecontext-compatibility/src/main/scala/org/apache/spark/sql/hive/HiveContext.scala

+   *
+   * @since 1.3.0
+   */
+  def refreshTable(tableName: String): Unit = {


if invalidateTable has different meaning than refreshTable, should we also add it to HiveContext? cc @yhuai

This class is for the compatibility purpose. Let's leave it as is.

andrewor14 · 2016-05-19T19:16:16Z

LGTM other than the renaming. We shouldn't have spark.catalog.refreshTable and spark.sessionState.refreshTable do different things. I would rename the latter to invalidateTable since that's what it's really doing.

gatorsmile · 2016-05-19T21:32:05Z

I see. spark.sessionState.invalidateTable already exists. They have the same implementation. Thus, I will just remove spark.sessionState.refreshTable? Let me do it

SparkQA · 2016-05-19T21:46:44Z

Test build #58905 has finished for PR 13156 at commit 20d5055.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2016-05-20T02:09:59Z

retest this please

SparkQA · 2016-05-20T03:12:03Z

Test build #58934 has finished for PR 13156 at commit 20d5055.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2016-05-20T04:19:54Z

retest this please

cloud-fan · 2016-05-20T04:38:21Z

LGTM, pending jenkins

gatorsmile · 2016-05-20T04:53:10Z

retest this please

SparkQA · 2016-05-20T06:32:54Z

Test build #58940 has finished for PR 13156 at commit 20d5055.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

#### What changes were proposed in this pull request? `refreshTable` was a method in `HiveContext`. It was deleted accidentally while we were migrating the APIs. This PR is to add it back to `HiveContext`. In addition, in `SparkSession`, we put it under the catalog namespace (`SparkSession.catalog.refreshTable`). #### How was this patch tested? Changed the existing test cases to use the function `refreshTable`. Also added a test case for refreshTable in `hivecontext-compatibility` Author: gatorsmile <gatorsmile@gmail.com> Closes #13156 from gatorsmile/refreshTable. (cherry picked from commit 39fd469) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

cloud-fan · 2016-05-20T06:39:19Z

thanks, merging to master and 2.0!

gatorsmile · 2016-05-20T06:41:43Z

Thank you!

gatorsmile added 5 commits May 17, 2016 13:38

initial fix

b1cd1c6

fix.

bdd7c61

fix again.

e3564d5

revert it back

c3f3f0b

Merge remote-tracking branch 'upstream/master' into refreshTable

9e6c4b7

yhuai reviewed May 17, 2016
View reviewed changes

gatorsmile added 2 commits May 17, 2016 20:47

address comments.

5f342f7

Merge remote-tracking branch 'upstream/master' into refreshTable

4ac3b76

viirya reviewed May 18, 2016
View reviewed changes

address comments

7142ef5

gatorsmile reviewed May 18, 2016
View reviewed changes

update the comment.

8a52ac6

cloud-fan reviewed May 19, 2016
View reviewed changes

remove the test

2b773b8

cloud-fan reviewed May 19, 2016
View reviewed changes

remove refreshTable

20d5055

asfgit closed this in 39fd469 May 20, 2016

gatorsmile mentioned this pull request Jun 30, 2016

[SPARK-16311][SQL] Improve metadata refresh #13989

Closed

Conversation

gatorsmile commented May 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 18, 2016

Uh oh!

SparkQA commented May 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented May 19, 2016

Uh oh!

SparkQA commented May 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented May 19, 2016

Uh oh!

gatorsmile commented May 19, 2016

Uh oh!

SparkQA commented May 19, 2016

Uh oh!

gatorsmile commented May 20, 2016

Uh oh!

SparkQA commented May 20, 2016

Uh oh!

gatorsmile commented May 20, 2016

Uh oh!

cloud-fan commented May 20, 2016

Uh oh!

gatorsmile commented May 20, 2016

Uh oh!

SparkQA commented May 20, 2016

Uh oh!

cloud-fan commented May 20, 2016

Uh oh!

gatorsmile commented May 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gatorsmile commented May 17, 2016 •

edited

Loading