You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by srowen <gi...@git.apache.org> on 2017/05/04 10:06:58 UTC

[GitHub] spark issue #17796: [SPARK-20519][SQL][CORE]Modify to prevent some possible ...

Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/17796
  
    @10110346  to be conservative, maybe remove the change to SQLContext. It's not clear to me that you are supposed to be able to call {{tables(null)}} because in this case you should call {{tables()}}. A NullPointerException seems like reasonable behavior.
    
    However I think you could also expand the improvement for `Utils.checkHost` and `checkHostPort`. They all take this "message" argument which is pretty pointless. The message is a little different in many places and doesn't include the actual value most of the time. Just setting a complete valid message once in the method rather than taking it as an argument would further help.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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