You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2019/05/27 18:06:18 UTC

[GitHub] [spark] rdblue commented on issue #24689: [SPARK-26946][SQL][FOLLOWUP] Handle lookupCatalog function not defined

rdblue commented on issue #24689: [SPARK-26946][SQL][FOLLOWUP] Handle lookupCatalog function not defined
URL: https://github.com/apache/spark/pull/24689#issuecomment-496277746
 
 
   @jzhuge, these changes look okay, but I think the right path is to always require a lookup function and get rid of the option. There is never a time when these should be used without a catalog lookup function and not requiring that one is supplied leads to avoidable errors.
   
   I've been testing a `FunctionCatalog` implementation and it was easy to forget to provide a catalog lookup. But you always need a catalog lookup that does something. You may want to default in some cases to throwing `NoSuchCatalogException`, but that depends on the context, not whether you forgot to supply the lookup.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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