srowen commented on pull request #29078: URL: https://github.com/apache/spark/pull/29078#issuecomment-657787280
Oh good catch, I think that's it. The issue is that `ListBuffer.toSeq` does _not_ simply return itself (not sure why). So this passes copies of the mutable ListBuffer which is later updated, so the component never sees the update. This is the kind of bug to look out for, to be sure - even in 2.12, I suppose, given this. I am slightly worried there's a case that tests don't catch. I manually reviewed this PR vs occurrences of ListBuffer and don't think there are more instances here, but will examine the previous PR too. I suspect this kind of thing will come up in a few places in the 2.13 build as .toSeq will in these cases most definitely make a copy of a mutable collection. Which is even good in a way, as we should update the code to be clearer about the fact that it's specifically a mutable Seq being passed -- but, will probably require more debugging in 2.13 later. ---------------------------------------------------------------- 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. 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
