[GitHub] [spark] Hisoka-X commented on a diff in pull request #41348: [SPARK-43203][SQL] Move all Drop Table case to DataSource V2

["Hisoka-X (via GitHub)" <gi...@apache.org>]

Hisoka-X commented on code in PR #41348:
URL: https://github.com/apache/spark/pull/41348#discussion_r1208726693


##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/IdentifierImpl.java:
##########
@@ -30,12 +30,12 @@
  *  An {@link Identifier} implementation.
  */
 @Evolving
-class IdentifierImpl implements Identifier {
+public class IdentifierImpl implements Identifier {

Review Comment:
   change scope to public, so can create it by test in https://github.com/apache/spark/pull/41348/files#diff-7ce7e7e03723d865798a6dcb6efa81aea25e925397e4c9288bbfd3317c8a515bR694-R698



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -849,6 +849,7 @@ class SessionCatalog(
         tempViews.remove(table)
       }
     }
+    invalidateCachedTable(name)

Review Comment:
   add `invalidateCachedTable`, it's logic which `DropTableCommand` will do in v1.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala:
##########
@@ -71,7 +71,12 @@ class V2SessionCatalog(catalog: SessionCatalog)
   }
 
   override def loadTable(ident: Identifier): Table = {
-    V1Table(catalog.getTableMetadata(ident.asTableIdentifier))
+    try {
+      V1Table(catalog.getTableMetadata(ident.asTableIdentifier))
+    } catch {
+      case _: NoSuchDatabaseException =>

Review Comment:
   loadTable will return `NoSuchDatabaseException`, not same as v1 behavior.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala:
##########
@@ -190,6 +191,11 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
         isSameName(ident.qualifier :+ ident.name) &&
           isSameName(v1Ident.catalog.toSeq ++ v1Ident.database :+ v1Ident.table)
 
+      case SubqueryAlias(ident, HiveTableRelation(catalogTable, _, _, _, _)) =>

Review Comment:
   Add support for `HiveTableRelation` in `isMatchedTableOrView`, so that can remove Cache normally when use hive catalog.



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