You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/17 08:49:33 UTC

[GitHub] [spark] MaxGekk opened a new pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

MaxGekk opened a new pull request #31575:
URL: https://github.com/apache/spark/pull/31575


   ### What changes were proposed in this pull request?
   1. Move parser tests from `DDLParserSuite` to `AlterTableRenameParserSuite`.
   2. Port DS v1 tests from `DDLSuite` and other test suites to `v1.AlterTableRenameBase` and to `v1.AlterTableRenameSuite`.
   3. Add a test for DSv2 `ALTER TABLE .. RENAME` to `v2.AlterTableRenameSuite`.
   
   ### Why are the changes needed?
   To improve test coverage.
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   By running new test suites:
   ```
   $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableRenameSuite"
   $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableRenameParserSuite"
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780557248


   **[Test build #135194 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135194/testReport)** for PR 31575 at commit [`c516011`](https://github.com/apache/spark/commit/c51601140b41dba2b7ca42368ebbd947835150cb).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class AlterTableRenameParserSuite extends AnalysisTest `
     * `trait AlterTableRenameSuiteBase extends QueryTest with DDLCommandTestUtils `
     * `trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase `
     * `class AlterTableRenameSuite extends command.AlterTableRenameSuiteBase with CommandSuiteBase `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780474824


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39775/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783282419


   **[Test build #135335 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135335/testReport)** for PR 31575 at commit [`ee548c4`](https://github.com/apache/spark/commit/ee548c48647f164b20dda3cce9d1c10734dcd26f).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783148659


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39915/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783165664


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39915/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577654041



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")
+      checkTables("ns", "dst_tbl")
+      checkAnswer(sql(s"SELECT c0 FROM $dst"), Seq(Row(0)))
+    }
+  }
+
+  test("destination database is different") {
+    withNamespaceAndTable("dst_ns", "dst_tbl") { dst =>
+      withNamespace("src_ns") {
+        sql(s"CREATE NAMESPACE $catalog.src_ns")
+        val src = dst.replace("dst", "src")
+        sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+        val errMsg = intercept[AnalysisException] {
+          sql(s"ALTER TABLE $src RENAME TO dst_ns.dst_tbl")
+        }.getMessage
+        assert(errMsg.contains("source and destination databases do not match"))
+      }
+    }
+  }
+
+  test("rename without explicitly specifying database") {

Review comment:
       Independently from `sql(s"USE $catalog.ns")`, v2 places `dst_tbl` directly to the catalog:
   ```
           sql(s"SHOW TABLES IN $catalog").show(false)
   
   +---------+---------+-----------+
   |namespace|tableName|isTemporary|
   +---------+---------+-----------+
   |         |dst_tbl  |false      |
   +---------+---------+-----------+
   ```
   but v1 keeps the table in the same namespace:
   ```
   +---------+---------+-----------+
   |namespace|tableName|isTemporary|
   +---------+---------+-----------+
   |ns       |dst_tbl  |false      |
   +---------+---------+-----------+
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783120465


   **[Test build #135335 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135335/testReport)** for PR 31575 at commit [`ee548c4`](https://github.com/apache/spark/commit/ee548c48647f164b20dda3cce9d1c10734dcd26f).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r578905026



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       The current behavior is different between v1 and v2. V1 follows the mainstream databases and `RENAME TO single_part_name` simply renames the table. V2 leaves the decision to the catalog implementation (the API doc doesn't explicitly define the behavior of moving between namespaces) and our testing in-memory catalog moves the table to the root namespace.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780407552


   **[Test build #135194 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135194/testReport)** for PR 31575 at commit [`c516011`](https://github.com/apache/spark/commit/c51601140b41dba2b7ca42368ebbd947835150cb).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-781576996


   @cloud-fan I opened the JIRA https://issues.apache.org/jira/browse/SPARK-34468 to adjust v2 behavior. Can we continue with this PR since it is about tests only.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780407552


   **[Test build #135194 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135194/testReport)** for PR 31575 at commit [`c516011`](https://github.com/apache/spark/commit/c51601140b41dba2b7ca42368ebbd947835150cb).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577744382



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       From end-user's perspective, it's terrible to force them to write `ALTER TABLE ns0.ns1.ns2.ns3.ns4.ns5.tbl RENAME TO ns0.ns1.ns2.ns3.ns4.ns5.tbl2` if they want to do a simple table rename ...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783182558


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39915/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783299605


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135335/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan closed pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31575:
URL: https://github.com/apache/spark/pull/31575


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r578317642



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       > Can you find another database that requires users to fully specify the qualifiers in RENAME?
   
   I have found yet. Some DBMSs don't allow to move a table to another schema/database (DB2, Oracle). MySQL allows to move to another schema/database (https://dev.mysql.com/doc/refman/8.0/en/rename-table.html). But I haven't found any DBMS which moves a table specified only via table name to the "root" namespace like v2 impl does for now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783182558


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39915/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577679216



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       I think it's intuitive to only rename the table instead of moving namespaces if we see `... RENAME TO single_part_name`. Spark should fill the "new name" with the namespace of the source table before calling the v2 API `TableCatalog.renameTable`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577632804



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")
+      checkTables("ns", "dst_tbl")
+      checkAnswer(sql(s"SELECT c0 FROM $dst"), Seq(Row(0)))
+    }
+  }
+
+  test("destination database is different") {
+    withNamespaceAndTable("dst_ns", "dst_tbl") { dst =>
+      withNamespace("src_ns") {
+        sql(s"CREATE NAMESPACE $catalog.src_ns")
+        val src = dst.replace("dst", "src")
+        sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+        val errMsg = intercept[AnalysisException] {
+          sql(s"ALTER TABLE $src RENAME TO dst_ns.dst_tbl")
+        }.getMessage
+        assert(errMsg.contains("source and destination databases do not match"))
+      }
+    }
+  }
+
+  test("rename without explicitly specifying database") {

Review comment:
       doesn't it work with v2 catalogs?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] rdblue commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r578800005



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       > This is actually my first proposal, but we need to define what's "correct identifier". Do you agree that "RENAME TO single_part_name" should only rename the table, but not moving the table to the root namespace?
   
   My expectation would be to rename the table without changing its namespace. My point was that whatever we decide, Spark should pass the correct full identifier to the catalog so that it is not up to the catalog to make that choice.
   
   What is the current behavior? Would changing this cause a breaking behavior change?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577739526



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       I believe the API should be simple and straightforward, and shouldn't add extra logic. If you need to rename a table in a namespace, please, specify full path.
   
   Also v1 has only one level of namespaces but v2 has many. And what if I want to move a table to another namespace like:
   ```
   ALTER TABLE ns0.ns1.ns2.ns3.ns4.ns5.tbl RENAME TO ns0.ns1.ns123.tbl
   ```
   v1 prohibits such moving as it allows to rename a table inside of the same database only.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] rdblue commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577771433



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       > we can simply update the API doc of TableCatalog.renameTable and say: if the newIdent has no namespace, it means the namespace of oldIdent
   
   -1 for this idea. I think this is a mistake.
   
   This would introduce ambiguity in the API that requires the implementer to know how to do something correctly. In DSv2, we avoid those cases by making Spark a little smarter so that we have uniform behavior across sources. Instead of adding documentation, Spark should always pass the correct identifier for the rename.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783075589


   https://github.com/apache/spark/pull/31594 is merged, @MaxGekk can you adjust this PR?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780479090


   @cloud-fan Could you review this PR, please.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577632044



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       doesn't it work with v2 catalogs?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577680869



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       Or we can simply update the API doc of `TableCatalog.renameTable` and say: if the `newIdent` has no namespace, it means the namespace of `oldIdent`. This is more backward compatible as it's a doc-only change.
   
   cc @rdblue 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577754402



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       From end-user's perspective, intuitive behavior should be like in file systems, I guess. `mv /ns0/ns1/ns2/ns3/ns4/ns5/tbl tbl2` will not place `tbl2` to `/ns0/ns1/ns2/ns3/ns4/ns5`, right?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r577649098



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       v2 places `dst_tbl` directly to the catalog:
   ```
         sql(s"SHOW TABLES IN $catalog").show(false)
   
   +---------+---------+-----------+
   |namespace|tableName|isTemporary|
   +---------+---------+-----------+
   |         |dst_tbl  |false      |
   +---------+---------+-----------+
   ```
   but v1 to the same namespace:
   ```
   +---------+---------+-----------+
   |namespace|tableName|isTemporary|
   +---------+---------+-----------+
   |ns       |dst_tbl  |false      |
   +---------+---------+-----------+
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783120465


   **[Test build #135335 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135335/testReport)** for PR 31575 at commit [`ee548c4`](https://github.com/apache/spark/commit/ee548c48647f164b20dda3cce9d1c10734dcd26f).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r578905026



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       The current behavior is different between v1 and v2. V1 follows the mainstream databases and `RENAME TO single_part_name` simply renames the table. V2 leaves the decision to the catalog implementation and our testing in-memory catalog moves the table to the root namespace.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783299605


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135335/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r579375661



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       Here is the fix for v2: https://github.com/apache/spark/pull/31594




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31575:
URL: https://github.com/apache/spark/pull/31575#discussion_r578136747



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableRenameSuite.scala
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.{AnalysisException, Row}
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. RENAME` command that check V1
+ * table catalogs. The tests that cannot run for all V1 catalogs are located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog: `org.apache.spark.sql.execution.command.v1.AlterTableRenameSuite`
+ *   - V1 Hive External catalog: `org.apache.spark.sql.hive.execution.command.AlterTableRenameSuite`
+ */
+trait AlterTableRenameSuiteBase extends command.AlterTableRenameSuiteBase {
+  test("omit namespace in the destination table") {
+    withNamespaceAndTable("ns", "dst_tbl") { dst =>
+      val src = dst.replace("dst", "src")
+      sql(s"CREATE TABLE $src (c0 INT) $defaultUsing")
+      sql(s"INSERT INTO $src SELECT 0")
+
+      sql(s"ALTER TABLE $src RENAME TO dst_tbl")

Review comment:
       @MaxGekk but SQL is not file system. Can you find another database that requires users to fully specify the qualifiers in RENAME?
   
   > Spark should always pass the correct identifier for the rename.
   
   @rdblue This is actually my first proposal, but we need to define what's "correct identifier". Do you agree that "RENAME TO single_part_name" should only rename the table, but not moving the table to the root namespace?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780572455


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135194/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780474824


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39775/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-780572455


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135194/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on pull request #31575: [SPARK-34450][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. RENAME tests

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31575:
URL: https://github.com/apache/spark/pull/31575#issuecomment-783195964


   thanks, merging to master!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org