Github user rdblue commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20647#discussion_r170303699
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/MicroBatchExecution.scala
 ---
    @@ -77,31 +79,32 @@ class MicroBatchExecution(
           
sparkSession.sqlContext.conf.disabledV2StreamingMicroBatchReaders.split(",")
     
         val _logicalPlan = analyzedPlan.transform {
    -      case streamingRelation@StreamingRelation(dataSourceV1, sourceName, 
output) =>
    -        toExecutionRelationMap.getOrElseUpdate(streamingRelation, {
    +      case s @ StreamingRelation(dsV1, sourceName, output) =>
    --- End diff --
    
    I don't think this counts as touching the code. This is near code you 
needed to change, but it makes the changes in this patch larger and more likely 
to conflict.
    
    Changes like this make maintenance much more difficult because they can 
cause conflicts that weren't necessary. After this change, commits would use 
`s` instead of `streamingRelation`, which would prevent any commit after this 
to be applied to a branch without this one. If that is avoidable because the 
change is not necessary, I want to avoid it.


---

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

Reply via email to