spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [spark] imback82 commented on a change in pull request #26684: [SPARK-30001][SQL] ResolveRelations should handle both V1 and V2 tables.
Date Thu, 05 Dec 2019 03:05:01 GMT
imback82 commented on a change in pull request #26684: [SPARK-30001][SQL] ResolveRelations
should handle both V1 and V2 tables.
URL: https://github.com/apache/spark/pull/26684#discussion_r354091426
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ##########
 @@ -776,34 +770,73 @@ class Analyzer(
       case _ => plan
     }
 
-    def apply(plan: LogicalPlan): LogicalPlan = ResolveTables(plan).resolveOperatorsUp {
-      case i @ InsertIntoStatement(u @ UnresolvedRelation(AsTableIdentifier(ident)), _, child,
_, _)
-          if child.resolved =>
-        EliminateSubqueryAliases(lookupTableFromCatalog(ident, u)) match {
+    def apply(plan: LogicalPlan): LogicalPlan = ResolveTempViews(plan).resolveOperatorsUp
{
+      case i @ InsertIntoStatement(
+          u @ UnresolvedRelation(SessionCatalogAndIdentifier(catalog, ident)), _, _, _, _)
+            if i.query.resolved =>
+        val relation = ResolveTempViews(u) match {
+          case unresolved: UnresolvedRelation =>
+            lookupRelation(catalog, ident, recurse = false).getOrElse(unresolved)
+          case tempView => tempView
+        }
+
+        EliminateSubqueryAliases(relation) match {
           case v: View =>
             u.failAnalysis(s"Inserting into a view is not allowed. View: ${v.desc.identifier}.")
           case other => i.copy(table = other)
         }
+
       case u: UnresolvedRelation => resolveRelation(u)
     }
 
-    // Look up the table with the given name from catalog. The database we used is decided
by the
-    // precedence:
-    // 1. Use the database part of the table identifier, if it is defined;
-    // 2. Use defaultDatabase, if it is defined(In this case, no temporary objects can be
used,
-    //    and the default database is only used to look up a view);
-    // 3. Use the currentDb of the SessionCatalog.
-    private def lookupTableFromCatalog(
-        tableIdentifier: TableIdentifier,
-        u: UnresolvedRelation,
-        defaultDatabase: Option[String] = None): LogicalPlan = {
-      val tableIdentWithDb = tableIdentifier.copy(
-        database = tableIdentifier.database.orElse(defaultDatabase))
-      try {
-        v1SessionCatalog.lookupRelation(tableIdentWithDb)
-      } catch {
-        case _: NoSuchTableException | _: NoSuchDatabaseException =>
-          u
+    // Look up a relation from a given session catalog with the following logic:
+    // 1) If a relation is not found in the catalog, return None.
+    // 2) If a relation is found,
+    //   a) if it is a v1 table not running on files, create a v1 relation
+    //   b) otherwise, create a v2 relation.
+    // 3) Otherwise, return None.
+    // If recurse is set to true, it will call `resolveRelation` recursively to resolve
+    // relations with the correct database scope.
+    private def lookupRelation(
+        catalog: CatalogPlugin,
+        ident: Identifier,
+        recurse: Boolean): Option[LogicalPlan] = {
+      val newIdent = withNewNamespace(ident)
+      assert(newIdent.namespace.size == 1)
+
+      CatalogV2Util.loadTable(catalog, newIdent) match {
+        case Some(v1Table: V1Table) =>
+          val tableIdent = TableIdentifier(newIdent.name, newIdent.namespace.headOption)
+          if (!isRunningDirectlyOnFiles(tableIdent)) {
 
 Review comment:
   Good point! For `isRunningDirectlyOnFiles` to be true, the table should not exist. If `CatalogV2Util.loadTable`
returned v1 table, it means that the table exists, so this will always be 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


With regards,
Apache Git Services

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


Mime
View raw message