You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ya...@apache.org on 2022/03/23 09:27:24 UTC

[spark] branch branch-3.3 updated: [SPARK-38587][SQL] Validating new location for rename command should use formatted names

This is an automated email from the ASF dual-hosted git repository.

yao pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.3 by this push:
     new ecc24c13 [SPARK-38587][SQL] Validating new location for rename command should use formatted names
ecc24c13 is described below

commit ecc24c13263f5e1a95c34b4bc58644a200dcf7cc
Author: Kent Yao <ya...@apache.org>
AuthorDate: Wed Mar 23 17:25:47 2022 +0800

    [SPARK-38587][SQL] Validating new location for rename command should use formatted names
    
    ### What changes were proposed in this pull request?
    
    Fix bug for `getDatabase` with a unformatted database name.
    
    ```java
    [info] - ALTER TABLE .. RENAME using V1 catalog V1 command: newName *** FAILED *** (61 milliseconds)
    [info]   org.apache.spark.sql.catalyst.analysis.NoSuchDatabaseException: Database 'CaseUpperCaseLower' not found
    [info]   at org.apache.spark.sql.catalyst.catalog.ExternalCatalog.requireDbExists(ExternalCatalog.scala:42)
    [info]   at org.apache.spark.sql.catalyst.catalog.ExternalCatalog.requireDbExists$(ExternalCatalog.scala:40)
    [info]   at org.apache.spark.sql.catalyst.catalog.InMemoryCatalog.requireDbExists(InMemoryCatalog.scala:47)
    [info]   at org.apache.spark.sql.catalyst.catalog.InMemoryCatalog.getDatabase(InMemoryCatalog.scala:171)
    [info]   at org.apache.spark.sql.catalyst.catalog.ExternalCatalogWithListener.getDatabase(ExternalCatalogWithListener.scala:65)
    [info]   at org.apache.spark.sql.catalyst.catalog.SessionCatalog.validateNewLocationOfRename(SessionCatalog.scala:1863)
    [info]   at org.apache.spark.sql.catalyst.catalog.SessionCatalog.renameTable(SessionCatalog.scala:739)
    [info]   at org.apache.spark.sql.execution.command.AlterTableRenameCommand.run(tables.scala:209
    ```
    
    ### Why are the changes needed?
    
    bugfix
    
    ### Does this PR introduce _any_ user-facing change?
    
    no, bugfix
    ### How was this patch tested?
    
    added new tests
    
    Closes #35895 from yaooqinn/SPARK-38587.
    
    Authored-by: Kent Yao <ya...@apache.org>
    Signed-off-by: Kent Yao <ya...@apache.org>
    (cherry picked from commit a3776e01839d7639e534bd345bcfb4bc63dd2a65)
    Signed-off-by: Kent Yao <ya...@apache.org>
---
 .../org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala | 10 ++++++----
 .../sql/execution/command/AlterTableRenameSuiteBase.scala      |  8 ++++++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
index 3727bb3..5872f2a 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
@@ -734,10 +734,9 @@ class SessionCatalog(
     } else {
       requireDbExists(db)
       if (oldName.database.isDefined || !tempViews.contains(oldTableName)) {
-        requireTableExists(TableIdentifier(oldTableName, Some(db)))
-        requireTableNotExists(TableIdentifier(newTableName, Some(db)))
         validateName(newTableName)
-        validateNewLocationOfRename(oldName, newName)
+        validateNewLocationOfRename(
+          TableIdentifier(oldTableName, Some(db)), TableIdentifier(newTableName, Some(db)))
         externalCatalog.renameTable(db, oldTableName, newTableName)
       } else {
         if (newName.database.isDefined) {
@@ -1856,10 +1855,13 @@ class SessionCatalog(
   private def validateNewLocationOfRename(
       oldName: TableIdentifier,
       newName: TableIdentifier): Unit = {
+    requireTableExists(oldName)
+    requireTableNotExists(newName)
     val oldTable = getTableMetadata(oldName)
     if (oldTable.tableType == CatalogTableType.MANAGED) {
+      assert(oldName.database.nonEmpty)
       val databaseLocation =
-        externalCatalog.getDatabase(oldName.database.getOrElse(currentDb)).locationUri
+        externalCatalog.getDatabase(oldName.database.get).locationUri
       val newTableLocation = new Path(new Path(databaseLocation), formatTableName(newName.table))
       val fs = newTableLocation.getFileSystem(hadoopConf)
       if (fs.exists(newTableLocation)) {
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableRenameSuiteBase.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableRenameSuiteBase.scala
index 1803ec0..2942d61 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableRenameSuiteBase.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableRenameSuiteBase.scala
@@ -136,4 +136,12 @@ trait AlterTableRenameSuiteBase extends QueryTest with DDLCommandTestUtils {
       checkAnswer(spark.table(dst), Row(1, 2))
     }
   }
+
+  test("SPARK-38587: use formatted names") {
+    withNamespaceAndTable("CaseUpperCaseLower", "CaseUpperCaseLower") { t =>
+      sql(s"CREATE TABLE ${t}_Old (i int) $defaultUsing")
+      sql(s"ALTER TABLE ${t}_Old RENAME TO CaseUpperCaseLower.CaseUpperCaseLower")
+      assert(spark.table(t).isEmpty)
+    }
+  }
 }

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