You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/15 07:13:26 UTC

[GitHub] [spark] HeartSaVioR commented on a diff in pull request #39069: [SPARK-41524][SS] Differentiate SQLConf and extraOptions in StateStor…

HeartSaVioR commented on code in PR #39069:
URL: https://github.com/apache/spark/pull/39069#discussion_r1049286005


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDB.scala:
##########
@@ -560,10 +560,11 @@ case class RocksDBConf(
 
 object RocksDBConf {
   /** Common prefix of all confs in SQLConf that affects RocksDB */
-  val ROCKSDB_CONF_NAME_PREFIX = "spark.sql.streaming.stateStore.rocksdb"
+  val ROCKSDB_SQL_CONF_NAME_PREFIX = "spark.sql.streaming.stateStore.rocksdb"
 
   private case class ConfEntry(name: String, default: String) {
-    def fullName: String = s"$ROCKSDB_CONF_NAME_PREFIX.${name}".toLowerCase(Locale.ROOT)
+    def sqlConfFullName: String = s"$ROCKSDB_SQL_CONF_NAME_PREFIX.${name}".toLowerCase(Locale.ROOT)
+    def lowerCaseName: String = name.toLowerCase(Locale.ROOT)

Review Comment:
   What about having different class for each case? It would be clearer to distinguish configs, e.g. we don't expect the same config to be set from both SQL conf and extraOptions. If we allow such case, it'll be a bit more complicated than what you proposed, e.g. we will have to define preference.



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