imback82 commented on a change in pull request #27642: [SPARK-30885][SQL] V1 
table name should be fully qualified if catalog name is provided
URL: https://github.com/apache/spark/pull/27642#discussion_r381817006
 
 

 ##########
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/LookupCatalog.scala
 ##########
 @@ -106,24 +107,29 @@ private[sql] trait LookupCatalog extends Logging {
 
     def unapply(nameParts: Seq[String]): Option[(CatalogPlugin, Identifier)] = 
{
       assert(nameParts.nonEmpty)
-      if (nameParts.length == 1) {
-        Some((currentCatalog, Identifier.of(catalogManager.currentNamespace, 
nameParts.head)))
+      val (catalog, ident) = if (nameParts.length == 1) {
+        (currentCatalog, Identifier.of(catalogManager.currentNamespace, 
nameParts.head))
       } else if (nameParts.head.equalsIgnoreCase(globalTempDB)) {
         // Conceptually global temp views are in a special reserved catalog. 
However, the v2 catalog
         // API does not support view yet, and we have to use v1 commands to 
deal with global temp
         // views. To simplify the implementation, we put global temp views in 
a special namespace
         // in the session catalog. The special namespace has higher priority 
during name resolution.
         // For example, if the name of a custom catalog is the same with 
`GLOBAL_TEMP_DATABASE`,
         // this custom catalog can't be accessed.
-        Some((catalogManager.v2SessionCatalog, nameParts.asIdentifier))
+        (catalogManager.v2SessionCatalog, nameParts.asIdentifier)
       } else {
         try {
-          Some((catalogManager.catalog(nameParts.head), 
nameParts.tail.asIdentifier))
+          (catalogManager.catalog(nameParts.head), nameParts.tail.asIdentifier)
         } catch {
           case _: CatalogNotFoundException =>
-            Some((currentCatalog, nameParts.asIdentifier))
+            (currentCatalog, nameParts.asIdentifier)
         }
       }
+      if (CatalogV2Util.isSessionCatalog(catalog) && ident.namespace.isEmpty) {
 
 Review comment:
   @cloud-fan For a session catalog, I could make this assumption that the 
namespace is required, right? I looked at `CatalogManager` that uses 
`v1SessionCatalog` for setting current namespace if the current catalog is a 
session catalog; and `v1SessionCatalog` requires the namespace (database) to 
already exist.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

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

Reply via email to