HeartSaVioR commented on issue #24647: [SPARK-27776][SQL]Avoid duplicate Java reflection in DataSource. URL: https://github.com/apache/spark/pull/24647#issuecomment-494273198 I'm not sure you're understanding the points here. Just assuming we don't want to change the current behavior (but open to performance boost), this patch is different from current for two points of view: 1) This patch will show different behavior if provider provides different instance per constructor call (this is actually obvious) and the instance, or DataSource leverages the fact. 2) This patch will show different behavior if instance is stateful, like calling method changes internal state. That might be what you already checked before (as you're saying all the implementations you've checked are stateless), but it's not guaranteed by interface contract. I'm not saying something is better and something is worse - just would like to let you understand the point what @srowen is saying (and what I've agreed). If this change brings enough value it might be possible to add the contract in interface, but I'm not the one to right to weigh the value. That's why I just only see the chance to refactor (extract the call to method) to simplify the code, nothing else.
---------------------------------------------------------------- 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 With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
