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 2022/06/23 21:23:29 UTC

[GitHub] [spark] amaliujia commented on a diff in pull request #36969: [SPARK-39235][SQL] make setCurrentDatabase compatible with 3 layer namespace

amaliujia commented on code in PR #36969:
URL: https://github.com/apache/spark/pull/36969#discussion_r905474007


##########
sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala:
##########
@@ -43,30 +43,47 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
 
   private def sessionCatalog: SessionCatalog = sparkSession.sessionState.catalog
 
-  private def requireDatabaseExists(dbName: String): Unit = {
-    if (!sessionCatalog.databaseExists(dbName)) {
-      throw QueryCompilationErrors.databaseDoesNotExistError(dbName)
-    }
-  }
-
   private def requireTableExists(dbName: String, tableName: String): Unit = {
     if (!sessionCatalog.tableExists(TableIdentifier(tableName, Some(dbName)))) {
       throw QueryCompilationErrors.tableDoesNotExistInDatabaseError(tableName, dbName)
     }
   }
 
+  // FIXME
+  private var currentDatabaseV2: Option[String] = None
+
   /**
    * Returns the current default database in this session.
    */
-  override def currentDatabase: String = sessionCatalog.getCurrentDatabase
+  override def currentDatabase: String =
+    currentDatabaseV2.getOrElse(sessionCatalog.getCurrentDatabase)
 
   /**
    * Sets the current default database in this session.
    */
   @throws[AnalysisException]("database does not exist")
   override def setCurrentDatabase(dbName: String): Unit = {
-    requireDatabaseExists(dbName)
-    sessionCatalog.setCurrentDatabase(dbName)
+    // `dbName` could be either a single database name (behavior in Spark 3.3 and prior) or
+    // a qualified namespace with catalog name. We assume it's a single database name
+    // and check if we can find the dbName in sessionCatalog. If so, we set the use old
+    // sessionCatalog APIs to set current database. Otherwise, we try 3-part name parsing and
+    // check if the database exists with catalogV2 APIs
+    if (sessionCatalog.databaseExists(dbName)) {

Review Comment:
   Probably no need to do all of this.
   
   You can use https://github.com/apache/spark/blob/a3fdf3b107b62089890a4246c168ca516dff5d44/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala#L106.
   
   You can access the catalog manager by `sparkSession.sessionState.catalogManager`



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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