You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jinxing64 <gi...@git.apache.org> on 2017/08/30 15:22:22 UTC
[GitHub] spark pull request #19086: [SPARK-21874][SQL] Support changing database when...
GitHub user jinxing64 opened a pull request:
https://github.com/apache/spark/pull/19086
[SPARK-21874][SQL] Support changing database when rename table.
## What changes were proposed in this pull request?
Support changing database of table by `alter table dbA.XXX rename to dbB.YYY`
## How was this patch tested?
Updated existing unit test.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jinxing64/spark SPARK-21874
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19086.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 #19086
----
commit 7865b3dbe5bc4cda263bc75f8c3524c7bb0fe81c
Author: jinxing <ji...@126.com>
Date: 2017-08-30T03:09:57Z
Support changing database when rename table.
----
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
@gatorsmile
I feel sorry if this pr breaks rules of current code. But I think the function is a good(convenient for user) one. In our warehouse, there hundreds ETLs are using this function provided by Hive.
I can keep working on this if you think it's worth :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136620936
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -495,15 +495,16 @@ private[hive] class HiveClientImpl(
shim.dropTable(client, dbName, tableName, true, ignoreIfNotExists, purge)
}
- override def alterTable(tableName: String, table: CatalogTable): Unit = withHiveState {
+ override def alterTable(dbName: String, tableName: String, table: CatalogTable): Unit =
+ withHiveState {
--- End diff --
This is not right. We should remove `tableName `.
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136723544
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -573,28 +573,29 @@ class SessionCatalog(
* If no database is specified, this will first attempt to rename a temporary table with
* the same name, then, if that does not exist, rename the table in the current database.
*
- * This assumes the database specified in `newName` matches the one in `oldName`.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(currentDb))
val oldTableName = formatTableName(oldName.table)
val newTableName = formatTableName(newName.table)
- if (db == globalTempViewManager.database) {
+ if (oldDb == globalTempViewManager.database || newDb == globalTempViewManager.database) {
+ if (oldDb != newDb) {
--- End diff --
Add comment above
> // when either old table or new table is a global temporary view
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
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 pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136643356
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -576,25 +576,25 @@ class SessionCatalog(
* This assumes the database specified in `newName` matches the one in `oldName`.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(oldDb))
--- End diff --
Please double check whether Hive behaves like this. Also add the test cases for 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 issue #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
@gatorsmile
I updated, let me known if there's still comments not resolved.
Thanks again for review.
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136745765
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -569,46 +569,51 @@ class SessionCatalog(
/**
* Rename a table.
*
- * If a database is specified in `oldName`, this will rename the table in that database.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
+ *
* If no database is specified, this will first attempt to rename a temporary table with
- * the same name, then, if that does not exist, rename the table in the current database.
+ * the same name, then, if that does not exist, current database will be used for locating
+ * `oldName` or `newName`.
--- End diff --
This is wrong, right? This is for `temp view`
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81260 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81260/testReport)** for PR 19086 at commit [`7865b3d`](https://github.com/apache/spark/commit/7865b3dbe5bc4cda263bc75f8c3524c7bb0fe81c).
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81374/
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 pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136724918
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala ---
@@ -418,6 +436,39 @@ abstract class SessionCatalogSuite extends AnalysisTest {
}
}
+ test("rename global temp table") {
+ withBasicCatalog { catalog =>
+ val globalTempTable = Range(1, 10, 2, 10)
+ catalog.createGlobalTempView("tbl1", globalTempTable, overrideIfExists = false)
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tbl1", "tbl2"))
+ // If database is not specified, global temp table will not be renamed
+ catalog.setCurrentDatabase("db1")
+ intercept[AnalysisException] {
+ catalog.renameTable(TableIdentifier("tbl1"), TableIdentifier("tblone"))
+ }
+ catalog.setCurrentDatabase("db2")
+ catalog.renameTable(TableIdentifier("tbl1"), TableIdentifier("tblone"))
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "tbl2"))
+ // Moving global temp table to another database is forbidden
+ intercept[AnalysisException] {
+ catalog.renameTable(
+ TableIdentifier("tbl1", Some("global_temp")), TableIdentifier("tbl3", Some("db2")))
+ }
+ // Moving table from database to be a global temp table is forbidden
+ intercept[AnalysisException] {
--- End diff --
Capture the error message and add an assert.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
@gatorsmile
Thanks for taking time look at this. I updated description.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81370 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81370/testReport)** for PR 19086 at commit [`d4a0f97`](https://github.com/apache/spark/commit/d4a0f97b9600dd661d81a79e71019a781eb812ff).
* 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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81277 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81277/testReport)** for PR 19086 at commit [`7e0954c`](https://github.com/apache/spark/commit/7e0954c2e32acbe3c5648afbba2856d0c6013f6b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class RenameTablePreEvent(`
* `case class RenameTableEvent(`
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136725012
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -573,28 +573,29 @@ class SessionCatalog(
* If no database is specified, this will first attempt to rename a temporary table with
* the same name, then, if that does not exist, rename the table in the current database.
*
- * This assumes the database specified in `newName` matches the one in `oldName`.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(currentDb))
val oldTableName = formatTableName(oldName.table)
val newTableName = formatTableName(newName.table)
- if (db == globalTempViewManager.database) {
+ if (oldDb == globalTempViewManager.database || newDb == globalTempViewManager.database) {
+ if (oldDb != newDb) {
+ throw new AnalysisException(
+ s"Cannot change database of table '$oldTableName'")
+ }
globalTempViewManager.rename(oldTableName, newTableName)
} else {
- requireDbExists(db)
if (oldName.database.isDefined || !tempTables.contains(oldTableName)) {
--- End diff --
change `else { if` to `else if(`.
Also add a comment here
> ` // when the old table is a regular table/view`
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136719337
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -502,17 +502,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
val storageWithNewPath = if (rawTable.tableType == MANAGED && hasPathOption) {
// If it's a managed table with path option and we are renaming it, then the path option
// becomes inaccurate and we need to update it according to the new table name.
- val newTablePath = defaultTablePath(TableIdentifier(newName, Some(db)))
+ val newTablePath = defaultTablePath(TableIdentifier(newName, Some(newDb)))
--- End diff --
https://github.com/apache/spark/pull/19086/files#diff-8c4108666a6639034f0ddbfa075bcb37R827
Is this ok?
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136745597
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -569,46 +569,51 @@ class SessionCatalog(
/**
* Rename a table.
*
- * If a database is specified in `oldName`, this will rename the table in that database.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
--- End diff --
`It will result` -> `it results`
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
yes, correct
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136619408
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -576,25 +576,25 @@ class SessionCatalog(
* This assumes the database specified in `newName` matches the one in `oldName`.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(oldDb))
--- End diff --
If database is not given, Hive moves into the current database.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81277/
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81374 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81374/testReport)** for PR 19086 at commit [`2adc205`](https://github.com/apache/spark/commit/2adc20526a30cadb08d673ac7fdf219f35d8268a).
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136640563
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -495,15 +495,16 @@ private[hive] class HiveClientImpl(
shim.dropTable(client, dbName, tableName, true, ignoreIfNotExists, purge)
}
- override def alterTable(tableName: String, table: CatalogTable): Unit = withHiveState {
+ override def alterTable(dbName: String, tableName: String, table: CatalogTable): Unit =
+ withHiveState {
--- End diff --
What's needed to change? Why can't the author works on it?
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
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 pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136724965
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala ---
@@ -418,6 +436,39 @@ abstract class SessionCatalogSuite extends AnalysisTest {
}
}
+ test("rename global temp table") {
+ withBasicCatalog { catalog =>
+ val globalTempTable = Range(1, 10, 2, 10)
+ catalog.createGlobalTempView("tbl1", globalTempTable, overrideIfExists = false)
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tbl1", "tbl2"))
+ // If database is not specified, global temp table will not be renamed
--- End diff --
`global temp table` -> `global temp views`. This is a general comment. We do not have `global temp table`
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81361 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81361/testReport)** for PR 19086 at commit [`cd00825`](https://github.com/apache/spark/commit/cd00825af7a1e3ef6fb7172fd9a70a2b9cb658d6).
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136725094
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -573,28 +573,29 @@ class SessionCatalog(
* If no database is specified, this will first attempt to rename a temporary table with
* the same name, then, if that does not exist, rename the table in the current database.
*
- * This assumes the database specified in `newName` matches the one in `oldName`.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(currentDb))
--- End diff --
Also add a comment to explain the behavior in the above function description.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81370 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81370/testReport)** for PR 19086 at commit [`d4a0f97`](https://github.com/apache/spark/commit/d4a0f97b9600dd661d81a79e71019a781eb812ff).
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136644147
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -502,17 +502,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
val storageWithNewPath = if (rawTable.tableType == MANAGED && hasPathOption) {
// If it's a managed table with path option and we are renaming it, then the path option
// becomes inaccurate and we need to update it according to the new table name.
- val newTablePath = defaultTablePath(TableIdentifier(newName, Some(db)))
+ val newTablePath = defaultTablePath(TableIdentifier(newName, Some(newDb)))
--- End diff --
Please add a test case to check whether Hive move the table directory crosses the database
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136609981
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -576,25 +576,25 @@ class SessionCatalog(
* This assumes the database specified in `newName` matches the one in `oldName`.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(oldDb))
--- End diff --
Hi, @jinxing64 . Is this correct? I guess that this should be `currentDb` instead of `oldDb`.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
After more discussion with others, this behavior change really bothers us. Both changes do not sound good to us. Where is the requirement from?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136610605
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala ---
@@ -90,10 +90,11 @@ private[hive] trait HiveClient {
def dropTable(dbName: String, tableName: String, ignoreIfNotExists: Boolean, purge: Boolean): Unit
/** Alter a table whose name matches the one specified in `table`, assuming it exists. */
- final def alterTable(table: CatalogTable): Unit = alterTable(table.identifier.table, table)
+ final def alterTable(table: CatalogTable): Unit =
+ alterTable(table.database, table.identifier.table, table)
- /** Updates the given table with new metadata, optionally renaming the table. */
- def alterTable(tableName: String, table: CatalogTable): Unit
+ /** Updates the give table in database with new metadata, optionally renaming the table. */
--- End diff --
The original `given` is correct.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
I'm from Meituan, a Chinese company
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81260/
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
Could you post the Hive compatibility in the PR description? Also please post the outputs from Hive command lines? 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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136620584
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -495,15 +495,16 @@ private[hive] class HiveClientImpl(
shim.dropTable(client, dbName, tableName, true, ignoreIfNotExists, purge)
}
- override def alterTable(tableName: String, table: CatalogTable): Unit = withHiveState {
+ override def alterTable(dbName: String, tableName: String, table: CatalogTable): Unit =
+ withHiveState {
--- End diff --
```Scala
override def alterTable(
dbName: String,
tableName: String,
table: CatalogTable): Unit = withHiveState {
```
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
> use db2;
> alter table db1.t2 rename to t1;
After this PR, it is renamed to `db2.t1`, right?
Before this PR, it is renamed to `db1.t1`, right?
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136251887
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalog.scala ---
@@ -131,13 +131,21 @@ abstract class ExternalCatalog
ignoreIfNotExists: Boolean,
purge: Boolean): Unit
- final def renameTable(db: String, oldName: String, newName: String): Unit = {
- postToAll(RenameTablePreEvent(db, oldName, newName))
- doRenameTable(db, oldName, newName)
- postToAll(RenameTableEvent(db, oldName, newName))
+ final def renameDbTable(
--- End diff --
No need to change the name.
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136747432
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -569,46 +569,51 @@ class SessionCatalog(
/**
* Rename a table.
*
- * If a database is specified in `oldName`, this will rename the table in that database.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
+ *
* If no database is specified, this will first attempt to rename a temporary table with
- * the same name, then, if that does not exist, rename the table in the current database.
+ * the same name, then, if that does not exist, current database will be used for locating
+ * `oldName` or `newName`.
--- End diff --
So should I make the comment here unchanged?
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
Please hold it. It means it is a behavior change. Let me consider it more.
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136724969
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -502,17 +502,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
val storageWithNewPath = if (rawTable.tableType == MANAGED && hasPathOption) {
// If it's a managed table with path option and we are renaming it, then the path option
// becomes inaccurate and we need to update it according to the new table name.
- val newTablePath = defaultTablePath(TableIdentifier(newName, Some(db)))
+ val newTablePath = defaultTablePath(TableIdentifier(newName, Some(newDb)))
--- End diff --
This is ok
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 closed the pull request at:
https://github.com/apache/spark/pull/19086
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136642064
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -495,15 +495,16 @@ private[hive] class HiveClientImpl(
shim.dropTable(client, dbName, tableName, true, ignoreIfNotExists, purge)
}
- override def alterTable(tableName: String, table: CatalogTable): Unit = withHiveState {
+ override def alterTable(dbName: String, tableName: String, table: CatalogTable): Unit =
+ withHiveState {
--- End diff --
https://github.com/apache/spark/pull/19104
Need to write a few test cases for it. It might take multiple passes for it.
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136724986
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala ---
@@ -357,25 +357,43 @@ abstract class SessionCatalogSuite extends AnalysisTest {
test("rename table") {
withBasicCatalog { catalog =>
+ catalog.setCurrentDatabase("db2")
assert(catalog.externalCatalog.listTables("db2").toSet == Set("tbl1", "tbl2"))
catalog.renameTable(TableIdentifier("tbl1", Some("db2")), TableIdentifier("tblone"))
assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "tbl2"))
catalog.renameTable(TableIdentifier("tbl2", Some("db2")), TableIdentifier("tbltwo"))
assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "tbltwo"))
- // Rename table without explicitly specifying database
+
+ // Current database will be used when rename table without explicitly specifying database
catalog.setCurrentDatabase("db2")
catalog.renameTable(TableIdentifier("tbltwo"), TableIdentifier("table_two"))
assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "table_two"))
- // Renaming "db2.tblone" to "db1.tblones" should fail because databases don't match
+
+ // Table will be moved across databases when its database is different from the destination.
+ catalog.renameTable(
+ TableIdentifier("table_two", None), TableIdentifier("tabletwo", Some("db1")))
+ assert(catalog.externalCatalog.listTables("db1").toSet == Set("tabletwo"))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone"))
+
+ catalog.renameTable(
+ TableIdentifier("tabletwo", Some("db1")), TableIdentifier("table_two", None))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "table_two"))
+
+ catalog.renameTable(
+ TableIdentifier("tblone", Some("db2")), TableIdentifier("tblone", Some("db1")))
+ assert(catalog.externalCatalog.listTables("db1").toSet == Set("tblone"))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("table_two"))
+
+ // Renaming "db2.tblone" to "db1.tblones" should fail because table cannot be found.
intercept[AnalysisException] {
--- End diff --
add an assert to check the message.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
Sure, current behavior is hive behavior.
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136723572
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -573,28 +573,29 @@ class SessionCatalog(
* If no database is specified, this will first attempt to rename a temporary table with
* the same name, then, if that does not exist, rename the table in the current database.
*
- * This assumes the database specified in `newName` matches the one in `oldName`.
+ * If the database specified in `newName` is different from the one specified in `oldName`,
+ * It will result in moving table across databases.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(currentDb))
val oldTableName = formatTableName(oldName.table)
val newTableName = formatTableName(newName.table)
- if (db == globalTempViewManager.database) {
+ if (oldDb == globalTempViewManager.database || newDb == globalTempViewManager.database) {
+ if (oldDb != newDb) {
+ throw new AnalysisException(
+ s"Cannot change database of table '$oldTableName'")
+ }
globalTempViewManager.rename(oldTableName, newTableName)
} else {
- requireDbExists(db)
if (oldName.database.isDefined || !tempTables.contains(oldTableName)) {
- requireTableExists(TableIdentifier(oldTableName, Some(db)))
- requireTableNotExists(TableIdentifier(newTableName, Some(db)))
+ requireDbExists(oldDb)
+ requireDbExists(newDb)
+ requireTableExists(TableIdentifier(oldTableName, Some(oldDb)))
+ requireTableNotExists(TableIdentifier(newTableName, Some(newDb)))
validateName(newTableName)
- externalCatalog.renameTable(db, oldTableName, newTableName)
+ externalCatalog.renameTable(oldDb, oldTableName, newDb, newTableName)
} else {
if (newName.database.isDefined) {
--- End diff --
Add a comment
> // when the old table is a non-global temporary view
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
Thanks for working on it! @jinxing64
Our current behavior is based on what the other traditional DBMS systems do. Moving tables across different database is rare. For example, DB2 and Oracle do not support it.
- https://docs.oracle.com/cd/B19306_01/server.102/b14200/statements_9019.htm
- https://www.ibm.com/support/knowledgecenter/SSEPGG_10.5.0/com.ibm.db2.luw.sql.ref.doc/doc/r0000980.html
So far, we do not have a clean way to support our current behavior and the Hive's behavior. Thus, we want to hold this until the community has more requests.
The quality of this PR is very close to the one we can merge. You can patch it to your local forked version.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
Thanks, I will refine soon.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
@gatorsmile More comments on this ?
Regarding the behavior change, should we follow Spark previous behavior or follow Hive? I'm ok with both.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136252314
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala ---
@@ -95,6 +95,9 @@ private[hive] trait HiveClient {
/** Updates the given table with new metadata, optionally renaming the table. */
def alterTable(tableName: String, table: CatalogTable): Unit
+ /** Updates the give table in database with new metadata, optionally renaming the table. */
+ def alterDbTable(dbName: String, tableName: String, table: CatalogTable): Unit
--- End diff --
Keep using `alterTable` instead of introducing a new one?
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136643478
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -576,25 +576,25 @@ class SessionCatalog(
* This assumes the database specified in `newName` matches the one in `oldName`.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(oldDb))
val oldTableName = formatTableName(oldName.table)
val newTableName = formatTableName(newName.table)
- if (db == globalTempViewManager.database) {
+ if (oldDb == globalTempViewManager.database || newDb == globalTempViewManager.database) {
+ if (oldDb != newDb) {
+ throw new AnalysisException(
+ s"Cannot change database of table '$oldTableName'")
--- End diff --
Add test cases for both scenarios?
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
It's not ok to follow Spark current behavior?(It will be different from Hive)
I make this pr because we are migrating from Hive to Spark and lots of our users are using this function.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136252051
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala ---
@@ -264,21 +264,22 @@ class InMemoryCatalog(
}
}
- override protected def doRenameTable(
- db: String,
+ override protected def doRenameDbTable(
+ oldDb: String,
oldName: String,
- newName: String): Unit = synchronized {
- requireTableExists(db, oldName)
- requireTableNotExists(db, newName)
- val oldDesc = catalog(db).tables(oldName)
- oldDesc.table = oldDesc.table.copy(identifier = TableIdentifier(newName, Some(db)))
-
+ newDbOpt: Option[String],
--- End diff --
It should be cleaner if you change `Option[String]` to `String`
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136724927
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala ---
@@ -418,6 +436,39 @@ abstract class SessionCatalogSuite extends AnalysisTest {
}
}
+ test("rename global temp table") {
+ withBasicCatalog { catalog =>
+ val globalTempTable = Range(1, 10, 2, 10)
+ catalog.createGlobalTempView("tbl1", globalTempTable, overrideIfExists = false)
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tbl1", "tbl2"))
+ // If database is not specified, global temp table will not be renamed
+ catalog.setCurrentDatabase("db1")
+ intercept[AnalysisException] {
--- End diff --
The same here.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81260 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81260/testReport)** for PR 19086 at commit [`7865b3d`](https://github.com/apache/spark/commit/7865b3dbe5bc4cda263bc75f8c3524c7bb0fe81c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class RenameDbTablePreEvent(`
* `case class RenameDbTableEvent(`
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136747159
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala ---
@@ -418,6 +439,42 @@ abstract class SessionCatalogSuite extends AnalysisTest {
}
}
+ test("rename global temp view") {
+ withBasicCatalog { catalog =>
+ val globalTempTable = Range(1, 10, 2, 10)
+ catalog.createGlobalTempView("tbl1", globalTempTable, overrideIfExists = false)
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tbl1", "tbl2"))
+ // If database is not specified, global temp view will not be renamed
+ catalog.setCurrentDatabase("db1")
+ val thrown1 = intercept[AnalysisException] {
+ catalog.renameTable(TableIdentifier("tbl1"), TableIdentifier("tblone"))
+ }
+ assert(thrown1.getMessage.contains("Table or view 'tbl1' not found in database 'db1'"))
+ catalog.setCurrentDatabase("db2")
+ catalog.renameTable(TableIdentifier("tbl1"), TableIdentifier("tblone"))
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "tbl2"))
+ // Moving global temp view to another database is forbidden
+ val thrown2 = intercept[AnalysisException] {
+ catalog.renameTable(
+ TableIdentifier("tbl1", Some("global_temp")), TableIdentifier("tbl3", Some("db2")))
+ }
+ assert(thrown2.getMessage.contains("Cannot change database of table 'tbl1'"))
+ // Moving table from regular database to be a global temp view is forbidden
+ val thrown3 = intercept[AnalysisException] {
+ catalog.renameTable(
+ TableIdentifier("tbl2", Some("db2")), TableIdentifier("tbltwo", Some("global_temp")))
+ }
--- End diff --
Normally, we put `.getMessage` here. Thus, we can make the next line shorter.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81374 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81374/testReport)** for PR 19086 at commit [`2adc205`](https://github.com/apache/spark/commit/2adc20526a30cadb08d673ac7fdf219f35d8268a).
* This patch **fails due to an unknown error code, -9**.
* 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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
Test cases are missing.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81361 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81361/testReport)** for PR 19086 at commit [`cd00825`](https://github.com/apache/spark/commit/cd00825af7a1e3ef6fb7172fd9a70a2b9cb658d6).
* 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 pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136643219
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -576,25 +576,25 @@ class SessionCatalog(
* This assumes the database specified in `newName` matches the one in `oldName`.
--- End diff --
Update the comments to reflect the new logics.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19086
If we follow what our current way, rename becomes a special case. All the other commands are following different resolution ways.
Just curious which company are your from? I am trying to see the impact of this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81361/
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 pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136724919
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala ---
@@ -418,6 +436,39 @@ abstract class SessionCatalogSuite extends AnalysisTest {
}
}
+ test("rename global temp table") {
+ withBasicCatalog { catalog =>
+ val globalTempTable = Range(1, 10, 2, 10)
+ catalog.createGlobalTempView("tbl1", globalTempTable, overrideIfExists = false)
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tbl1", "tbl2"))
+ // If database is not specified, global temp table will not be renamed
+ catalog.setCurrentDatabase("db1")
+ intercept[AnalysisException] {
+ catalog.renameTable(TableIdentifier("tbl1"), TableIdentifier("tblone"))
+ }
+ catalog.setCurrentDatabase("db2")
+ catalog.renameTable(TableIdentifier("tbl1"), TableIdentifier("tblone"))
+ assert(catalog.getGlobalTempView("tbl1") == Option(globalTempTable))
+ assert(catalog.externalCatalog.listTables("db2").toSet == Set("tblone", "tbl2"))
+ // Moving global temp table to another database is forbidden
+ intercept[AnalysisException] {
--- End diff --
The same here.
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19086
**[Test build #81277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81277/testReport)** for PR 19086 at commit [`7e0954c`](https://github.com/apache/spark/commit/7e0954c2e32acbe3c5648afbba2856d0c6013f6b).
---
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 #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/19086
@gatorsmile
OK and thanks a lot for review :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19086: [SPARK-21874][SQL] Support changing database when rename...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81370/
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 pull request #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136252223
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -576,25 +576,25 @@ class SessionCatalog(
* This assumes the database specified in `newName` matches the one in `oldName`.
*/
def renameTable(oldName: TableIdentifier, newName: TableIdentifier): Unit = synchronized {
- val db = formatDatabaseName(oldName.database.getOrElse(currentDb))
- newName.database.map(formatDatabaseName).foreach { newDb =>
- if (db != newDb) {
- throw new AnalysisException(
- s"RENAME TABLE source and destination databases do not match: '$db' != '$newDb'")
- }
- }
+ val oldDb = formatDatabaseName(oldName.database.getOrElse(currentDb))
+ val newDb = formatDatabaseName(newName.database.getOrElse(oldDb))
val oldTableName = formatTableName(oldName.table)
val newTableName = formatTableName(newName.table)
- if (db == globalTempViewManager.database) {
+ if (oldDb == globalTempViewManager.database) {
--- End diff --
`newDB` should not be `globalTempViewManager.database`. Temporary views or global temporary views should be inapplicable to move from one DB to another DB
---
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 #19086: [SPARK-21874][SQL] Support changing database when...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19086#discussion_r136623529
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -495,15 +495,16 @@ private[hive] class HiveClientImpl(
shim.dropTable(client, dbName, tableName, true, ignoreIfNotExists, purge)
}
- override def alterTable(tableName: String, table: CatalogTable): Unit = withHiveState {
+ override def alterTable(dbName: String, tableName: String, table: CatalogTable): Unit =
+ withHiveState {
--- End diff --
Let me change the impl of HiveClientImpl. Then, you can work on it.
---
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