[GitHub] [incubator-kyuubi] iodone commented on a change in pull request #1967: [KYUUBI #1963] Make plan only mode skippable for configuable plans

[GitBox <gi...@apache.org>]

iodone commented on a change in pull request #1967:
URL: https://github.com/apache/incubator-kyuubi/pull/1967#discussion_r813503450



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/PlanOnlyStatement.scala
##########
@@ -46,24 +51,11 @@ class PlanOnlyStatement(
     } else result.schema
   }
 
-  private def shouldDirectRun(plan: LogicalPlan): Boolean = {
-    val className = plan.getClass.getSimpleName
-    className == "SetCommand" ||
-    className == "ResetCommand" ||
-    className == "UseStatement" ||
-    className == "SetNamespaceCommand" ||
-    className == "CacheTableStatement" ||
-    className == "CacheTableCommand" ||
-    className == "CacheTableAsSelect" ||
-    className == "CreateViewStatement" ||
-    className == "CreateViewCommand"
-  }
-
   override protected def runInternal(): Unit = {
     try {
       val parsed = spark.sessionState.sqlParser.parsePlan(statement)
       parsed match {
-        case cmd if shouldDirectRun(cmd) =>
+        case cmd if planExcludes.contains(cmd.getClass.getSimpleName) =>

Review comment:
       IMO, the configuration here is to provide the exact className, if you want to skip cacheTable,
   `CacheTableStatement` in spark 3.0 and `CacheTableCommand` in Spark 3.2, it's too hard for users to set the right options. Could the options here be more generic, like set/reset/cacheTable/CreateTempView, rather than requiring the actual class name?




-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org