[GitHub] [incubator-kyuubi] cfmcgrady commented on a diff in pull request #3988: Final stage config isolation support write only

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

cfmcgrady commented on code in PR #3988:
URL: https://github.com/apache/incubator-kyuubi/pull/3988#discussion_r1051970910


##########
extensions/spark/kyuubi-extension-spark-common/src/main/scala/org/apache/kyuubi/sql/KyuubiQueryStagePreparation.scala:
##########
@@ -58,6 +58,16 @@ case class FinalStageConfigIsolation(session: SparkSession) extends Rule[SparkPl
     }
 
     if (isFinalStage(plan)) {
+      // We can not get the whole plan at query preparation phase to detect if current plan is
+      // for writing, so we depend on a tag which is been injected at post resolution phase.
+      // Note: we should still do clean up previous config for non-final stage to avoid such case:
+      // the first statement is write, but the second statement is query.
+      if (conf.getConf(FINAL_STAGE_CONFIG_ISOLATION_WRITE_ONLY) &&
+        conf.getConf(KyuubiSQLConf.MARK_NUM_OUTPUT_COLUMNS) &&

Review Comment:
   Rule `MarkNumOutputColumnsRule` doesn't make any sense when condition `FINAL_STAGE_CONFIG_ISOLATION_WRITE_ONLY` is false, maybe we can simplify these two config?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org