[GitHub] [spark] dongjoon-hyun commented on pull request #41472: [SPARK-43976][CORE] Handle the case where modifiedConfigs doesn't exist in event logs

[via GitHub]

dongjoon-hyun commented on PR #41472:
URL: https://github.com/apache/spark/pull/41472#issuecomment-1579096582

   Got it. I double-checked and found more usages
   ```
   $ git grep '\.modifiedConfigs' | grep -v test | grep -v sessionState
   
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/ExecutionPage.scala:  
        executionUIData.modifiedConfigs.filterKeys(
   
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/ExecutionPage.scala:  
        
executionUIData.modifiedConfigs.filterKeys(_.startsWith(pandasOnSparkConfPrefix)).toMap)
   
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala:
          executionData.modifiedConfigs = sqlStoreData.modifiedConfigs
   
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala:
    exec.modifiedConfigs = modifiedConfigs
   
sql/core/src/main/scala/org/apache/spark/status/protobuf/sql/SQLExecutionUIDataSerializer.scala:
    if (ui.modifiedConfigs != null) {
   
sql/core/src/main/scala/org/apache/spark/status/protobuf/sql/SQLExecutionUIDataSerializer.scala:
      ui.modifiedConfigs.foreach {
   ```
   
   1. It's read from `sqlStoreData.modifiedConfigs`.
   
https://github.com/apache/spark/blob/8c6a54d70a79ecbbca45610b5d7dbf788e1e696d/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala#L97
   
   2. It's read from event.
   
https://github.com/apache/spark/blob/8c6a54d70a79ecbbca45610b5d7dbf788e1e696d/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala#L344-L345
   
   3. It's assigned manually.
   
https://github.com/apache/spark/blob/8c6a54d70a79ecbbca45610b5d7dbf788e1e696d/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala#L363
   
   In case of `SQLExecutionUIDataSerializer`, it loads from protobuf and it 
used `null` check.
   
   4. 
https://github.com/apache/spark/blob/8c6a54d70a79ecbbca45610b5d7dbf788e1e696d/sql/core/src/main/scala/org/apache/spark/status/protobuf/sql/SQLExecutionUIDataSerializer.scala#L88
   
   5. 
https://github.com/apache/spark/blob/8c6a54d70a79ecbbca45610b5d7dbf788e1e696d/sql/core/src/main/scala/org/apache/spark/status/protobuf/sql/SQLExecutionUIDataSerializer.scala#L37-L41
   
   It seems that we have various ways to create `SQLExecutionUIData`. Given 
that, UI change is the minimal one.


-- 
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-unsubscr...@spark.apache.org

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


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