You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2017/01/09 08:01:12 UTC
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/16514
[SPARK-19128] [SQL] Refresh Cache after Set Location
### What changes were proposed in this pull request?
```Scala
val catalog = spark.sessionState.catalog
sql("CREATE TABLE tab1 using parquet AS SELECT 1 as a")
sql("CREATE TABLE tab2 using parquet AS SELECT 2 as a")
checkAnswer(spark.table("tab1"), Seq(Row(1)))
checkAnswer(spark.table("tab2"), Seq(Row(2)))
val metadataTab1 = catalog.getTableMetadata(TableIdentifier("tab1"))
val locTab1 = metadataTab1.storage.locationUri
sql(s"ALTER TABLE tab2 SET LOCATION '${locTab1.get}'")
spark.table("tab2").show()
```
The above example will output incorrect the results. The reason is the cached relation's location still points to the old location.
Thus, this PR is to refresh the cache after `SET LOCATION`.
### How was this patch tested?
Added test cases into HiveDDLSuite.scala
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark refreshSetLocation
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16514.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #16514
----
commit fa49848cee21a05c02466295a39d04fd73470687
Author: gatorsmile <ga...@gmail.com>
Date: 2017-01-09T07:17:30Z
fix.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95112018
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -555,6 +557,61 @@ class HiveDDLSuite
}
}
+ test("Alter Table Set Location") {
+ withTable("tab1", "tab2") {
+ val catalog = spark.sessionState.catalog
+ sql("CREATE TABLE tab1 using parquet AS SELECT 1 as a")
+ sql("CREATE TABLE tab2 using parquet AS SELECT 2 as a")
+ checkAnswer(spark.table("tab1"), Seq(Row(1)))
+ checkAnswer(spark.table("tab2"), Seq(Row(2)))
+ val metadataTab1 = catalog.getTableMetadata(TableIdentifier("tab1"))
+ val locTab1 = metadataTab1.storage.locationUri
+ val metadataTab2 = catalog.getTableMetadata(TableIdentifier("tab2"))
+ val locTab2 = metadataTab2.storage.locationUri
+ assert(locTab1.isDefined)
+ assert(locTab2.isDefined)
+ try {
+ sql(s"ALTER TABLE tab2 SET LOCATION '${locTab1.get}'")
+ checkAnswer(spark.table("tab2"), Seq(Row(1)))
+ } finally {
+ val root = new Path(locTab2.get)
+ val fs = root.getFileSystem(spark.sparkContext.hadoopConfiguration)
+ fs.delete(root, true)
+ }
+ }
+ }
+
+ test("Alter Table Rename Partition") {
--- End diff --
Although we do not refresh the cache, we still can get the latest location of the table partition from the external catalog by calling [listPartitionsByFilter](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/CatalogFileIndex.scala#L69). We are not using the cached data for partition locations.
Adding this test case is just to ensure we do not break this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95112371
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -555,6 +557,61 @@ class HiveDDLSuite
}
}
+ test("Alter Table Set Location") {
+ withTable("tab1", "tab2") {
+ val catalog = spark.sessionState.catalog
+ sql("CREATE TABLE tab1 using parquet AS SELECT 1 as a")
+ sql("CREATE TABLE tab2 using parquet AS SELECT 2 as a")
+ checkAnswer(spark.table("tab1"), Seq(Row(1)))
+ checkAnswer(spark.table("tab2"), Seq(Row(2)))
+ val metadataTab1 = catalog.getTableMetadata(TableIdentifier("tab1"))
+ val locTab1 = metadataTab1.storage.locationUri
+ val metadataTab2 = catalog.getTableMetadata(TableIdentifier("tab2"))
+ val locTab2 = metadataTab2.storage.locationUri
+ assert(locTab1.isDefined)
+ assert(locTab2.isDefined)
+ try {
+ sql(s"ALTER TABLE tab2 SET LOCATION '${locTab1.get}'")
+ checkAnswer(spark.table("tab2"), Seq(Row(1)))
+ } finally {
+ val root = new Path(locTab2.get)
+ val fs = root.getFileSystem(spark.sparkContext.hadoopConfiguration)
+ fs.delete(root, true)
+ }
+ }
+ }
+
+ test("Alter Table Rename Partition") {
+ withTempView("tempView1") {
+ withTable("partitionedTab") {
+ spark.range(2).select('id as 'a, 'id as 'b).createTempView("tempView1")
+ sql(
+ """
+ |CREATE TABLE partitionedTab
+ |USING parquet
+ |PARTITIONED BY (b)
+ |AS SELECT a, b from tempView1
+ """.stripMargin)
+ // fill the cache
+ checkAnswer(spark.table("partitionedTab"), Seq(Row(0, 0), Row(1, 1)))
+
+ sql("ALTER TABLE partitionedTab PARTITION (b=1) RENAME TO PARTITION (b=3)")
+ checkAnswer(spark.table("partitionedTab"), Seq(Row(0, 0), Row(1, 3)))
+ }
+ }
+ }
+
+ test("Alter Table Rename Table") {
--- End diff --
The rename table command processing already called `refreshTable`. Thus, this is just to cover the test case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95287862
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -119,7 +119,31 @@ private[hive] class HiveMetastoreCatalog(sparkSession: SparkSession) extends Log
qualifiedTableName.database, qualifiedTableName.name)
if (DDLUtils.isDatasourceTable(table)) {
- val dataSourceTable = cachedDataSourceTables(qualifiedTableName)
+ val dataSourceTable =
+ cachedDataSourceTables(qualifiedTableName) match {
+ case l @ LogicalRelation(relation: HadoopFsRelation, _, _) =>
+ val location = table.storage.locationUri
+ val cachedLocation = relation.location.rootPaths
+ val isSamePath = location.isDefined && cachedLocation.size == 1 &&
--- End diff --
The size of `cachedLocation` is always 1, because we rely on [resolveRelation](https://github.com/gatorsmile/spark/blob/f6b88e6595271dbd6c868f7e220ba480872e1a9f/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L88) to build the cached LogicalRelation.
In the resolveRelation, the [rootPaths is always pointing to a single file](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSource.scala#L368).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71091 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71091/testReport)** for PR 16514 at commit [`d7cd667`](https://github.com/apache/spark/commit/d7cd667fa7ffdcccb1470c1e2ef6087d5e60f6b3).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71610/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71091/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/16514
That code path is for get the cache for the convertible Hive table. We have another [code path to access the cache](https://github.com/apache/spark/blob/24482858e05bea84cacb41c62be0a9aaa33897ee/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L122) for native data source tables. I think the reason why we do not add the extra checks is we assume native data source tables are completely managed by Spark
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:
https://github.com/apache/spark/pull/16514
I see. What do you think about adding that check in the caching code rather than require invalidation calls?
After all, the SET LOCATION may be issued by a separate Spark cluster connecting to the same hive metastore, so you'd need to invalidate all the cache of all clients talking to the metastore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95489960
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -119,7 +119,30 @@ private[hive] class HiveMetastoreCatalog(sparkSession: SparkSession) extends Log
qualifiedTableName.database, qualifiedTableName.name)
if (DDLUtils.isDatasourceTable(table)) {
- val dataSourceTable = cachedDataSourceTables(qualifiedTableName)
+ val dataSourceTable =
+ cachedDataSourceTables(qualifiedTableName) match {
+ case l @ LogicalRelation(relation: HadoopFsRelation, _, _) =>
+ // Ignore the scheme difference when comparing the paths
+ val isSamePath =
+ table.storage.locationUri.isDefined && relation.location.rootPaths.size == 1 &&
+ table.storage.locationUri.get == relation.location.rootPaths.head.toUri.getPath
+ // If we have the same paths, same schema, and same partition spec,
+ // we will use the cached relation.
+ val useCached =
+ isSamePath &&
+ l.schema == table.schema &&
+ relation.bucketSpec == table.bucketSpec &&
+ relation.partitionSchema == table.partitionSchema
+ if (useCached) {
+ l
+ } else {
+ // If the cached relation is not updated, we invalidate it right away.
+ cachedDataSourceTables.invalidate(qualifiedTableName)
+ // Reload it from the external catalog
+ cachedDataSourceTables(qualifiedTableName)
+ }
+ case o => o
+ }
--- End diff --
Understood.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71109 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71109/testReport)** for PR 16514 at commit [`0805cf6`](https://github.com/apache/spark/commit/0805cf6300953ac813ca9d10b76948a42af02a95).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/16514
A good suggestion. Will do the code changes tomorrow. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile closed the pull request at:
https://github.com/apache/spark/pull/16514
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71109/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95258103
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -754,6 +754,7 @@ case class AlterTableSetLocationCommand(
// No partition spec is specified, so we set the location for the table itself
catalog.alterTable(table.withNewStorage(locationUri = Some(location)))
}
+
--- End diff --
Will remove this later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71108 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71108/testReport)** for PR 16514 at commit [`f6b88e6`](https://github.com/apache/spark/commit/f6b88e6595271dbd6c868f7e220ba480872e1a9f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71610 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71610/testReport)** for PR 16514 at commit [`0805cf6`](https://github.com/apache/spark/commit/0805cf6300953ac813ca9d10b76948a42af02a95).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95258292
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -119,7 +119,30 @@ private[hive] class HiveMetastoreCatalog(sparkSession: SparkSession) extends Log
qualifiedTableName.database, qualifiedTableName.name)
if (DDLUtils.isDatasourceTable(table)) {
- val dataSourceTable = cachedDataSourceTables(qualifiedTableName)
+ val dataSourceTable =
+ cachedDataSourceTables(qualifiedTableName) match {
+ case l @ LogicalRelation(relation: HadoopFsRelation, _, _) =>
+ // Ignore the scheme difference when comparing the paths
+ val isSamePath =
+ table.storage.locationUri.isDefined && relation.location.rootPaths.size == 1 &&
+ table.storage.locationUri.get == relation.location.rootPaths.head.toUri.getPath
--- End diff --
Let me do more checks to see whether `rootPaths` could have more than one path when the table is partitioned.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71069/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/16514
> A good suggestion. Will do the code changes tomorrow. Thanks!
I look forward to seeing this. Thanks for taking this on.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71109 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71109/testReport)** for PR 16514 at commit [`0805cf6`](https://github.com/apache/spark/commit/0805cf6300953ac813ca9d10b76948a42af02a95).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71108 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71108/testReport)** for PR 16514 at commit [`f6b88e6`](https://github.com/apache/spark/commit/f6b88e6595271dbd6c868f7e220ba480872e1a9f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71108/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71069 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71069/testReport)** for PR 16514 at commit [`fa49848`](https://github.com/apache/spark/commit/fa49848cee21a05c02466295a39d04fd73470687).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95490619
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -555,6 +557,61 @@ class HiveDDLSuite
}
}
+ test("Alter Table Set Location") {
+ withTable("tab1", "tab2") {
+ val catalog = spark.sessionState.catalog
+ sql("CREATE TABLE tab1 using parquet AS SELECT 1 as a")
+ sql("CREATE TABLE tab2 using parquet AS SELECT 2 as a")
+ checkAnswer(spark.table("tab1"), Seq(Row(1)))
+ checkAnswer(spark.table("tab2"), Seq(Row(2)))
+ val metadataTab1 = catalog.getTableMetadata(TableIdentifier("tab1"))
+ val locTab1 = metadataTab1.storage.locationUri
+ val metadataTab2 = catalog.getTableMetadata(TableIdentifier("tab2"))
+ val locTab2 = metadataTab2.storage.locationUri
+ assert(locTab1.isDefined)
+ assert(locTab2.isDefined)
+ try {
+ sql(s"ALTER TABLE tab2 SET LOCATION '${locTab1.get}'")
+ checkAnswer(spark.table("tab2"), Seq(Row(1)))
+ } finally {
+ val root = new Path(locTab2.get)
+ val fs = root.getFileSystem(spark.sparkContext.hadoopConfiguration)
+ fs.delete(root, true)
--- End diff --
Why aren't we deleting `locTab1`, too?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95928814
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -555,6 +557,61 @@ class HiveDDLSuite
}
}
+ test("Alter Table Set Location") {
+ withTable("tab1", "tab2") {
+ val catalog = spark.sessionState.catalog
+ sql("CREATE TABLE tab1 using parquet AS SELECT 1 as a")
+ sql("CREATE TABLE tab2 using parquet AS SELECT 2 as a")
+ checkAnswer(spark.table("tab1"), Seq(Row(1)))
+ checkAnswer(spark.table("tab2"), Seq(Row(2)))
+ val metadataTab1 = catalog.getTableMetadata(TableIdentifier("tab1"))
+ val locTab1 = metadataTab1.storage.locationUri
+ val metadataTab2 = catalog.getTableMetadata(TableIdentifier("tab2"))
+ val locTab2 = metadataTab2.storage.locationUri
+ assert(locTab1.isDefined)
+ assert(locTab2.isDefined)
+ try {
+ sql(s"ALTER TABLE tab2 SET LOCATION '${locTab1.get}'")
+ checkAnswer(spark.table("tab2"), Seq(Row(1)))
+ } finally {
+ val root = new Path(locTab2.get)
+ val fs = root.getFileSystem(spark.sparkContext.hadoopConfiguration)
+ fs.delete(root, true)
--- End diff --
@mallman After altering the table location, `locTab1` is not the location of the `tab2`. Thus, when we dropping the table `tab2`, the location is not automaticaly deleted. Here, we need to manually delete it; otherwise, the following test case is unable to create a managed table using the name `tab1`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16514: [SPARK-19128] [SQL] Refresh Cache after Set Locat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16514#discussion_r95247659
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -119,7 +119,30 @@ private[hive] class HiveMetastoreCatalog(sparkSession: SparkSession) extends Log
qualifiedTableName.database, qualifiedTableName.name)
if (DDLUtils.isDatasourceTable(table)) {
- val dataSourceTable = cachedDataSourceTables(qualifiedTableName)
+ val dataSourceTable =
+ cachedDataSourceTables(qualifiedTableName) match {
+ case l @ LogicalRelation(relation: HadoopFsRelation, _, _) =>
+ // Ignore the scheme difference when comparing the paths
+ val isSamePath =
+ table.storage.locationUri.isDefined && relation.location.rootPaths.size == 1 &&
+ table.storage.locationUri.get == relation.location.rootPaths.head.toUri.getPath
+ // If we have the same paths, same schema, and same partition spec,
+ // we will use the cached relation.
+ val useCached =
+ isSamePath &&
+ l.schema == table.schema &&
+ relation.bucketSpec == table.bucketSpec &&
+ relation.partitionSchema == table.partitionSchema
+ if (useCached) {
+ l
+ } else {
+ // If the cached relation is not updated, we invalidate it right away.
+ cachedDataSourceTables.invalidate(qualifiedTableName)
+ // Reload it from the external catalog
+ cachedDataSourceTables(qualifiedTableName)
+ }
+ case o => o
+ }
--- End diff --
The above fix is for ensuring the fix is small enough to be backported to the 2.1 branch.
In the master branch, I tried to combine it with the existing [`getCached`](https://github.com/gatorsmile/spark/blob/d7cd667fa7ffdcccb1470c1e2ef6087d5e60f6b3/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L164-L212). However, it requires refactoring. Thus, it will be submitted as a separate PR. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71610 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71610/testReport)** for PR 16514 at commit [`0805cf6`](https://github.com/apache/spark/commit/0805cf6300953ac813ca9d10b76948a42af02a95).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:
https://github.com/apache/spark/pull/16514
Do you know why this check in the relation cache that the root paths have not changed is not sufficient? https://github.com/apache/spark/blob/24482858e05bea84cacb41c62be0a9aaa33897ee/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L160
Seems SET LOCATION should cause that to change, causing the next cache get to invalidate the relation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/16514
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16514
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16514: [SPARK-19128] [SQL] Refresh Cache after Set Location
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16514
**[Test build #71091 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71091/testReport)** for PR 16514 at commit [`d7cd667`](https://github.com/apache/spark/commit/d7cd667fa7ffdcccb1470c1e2ef6087d5e60f6b3).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org