[ 
https://issues.apache.org/jira/browse/HIVE-5400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13784737#comment-13784737
 ] 

Brock Noland commented on HIVE-5400:
------------------------------------

Edward,

Thank you very much for taking a look at this patch!

bq. Would we be better or returning CommandProcessorResponse's 

Several of the execution methods will do further processing before invoking the 
processor so I don't feel we should invoke the processor and then return the 
result. Additionally in general it's common to use a factory to get the 
"executor" and then call the "executor" in the callers context as it sees fit. 
This approach fits that paradigm.

bq. or throwing a new type of exception? I am struggling to rationalize 
SQLExceptions thrown from this part of hive code.

I certainly empathize with your feeling but I don't feel Hive we have the 
correct hierarchy to implement anything "better" at this point. In regards to 
throwing a SQLException I don't see what the advantage of throwing a separate 
exception would be. We are indicating the correct SQL State. But more 
importantly two callers are only going to log it regardless of the exception 
type and the third (HS2) is simply going to convert it to a HiveSQLException. 
I'd prefer to throw a HiveSQLException but that is a member of the service 
module and since service depends on ql it would create a circular dependency. 

If at some point in the future we create additional layers of abstraction 
between the "tool" interface and the execution implementation then I could see 
an improved exception hierarchy. However, I don't feel that is a blocker for 
this patch. If there are concrete suggestions on this aspect I think it'd be a 
good use of a follow-on JIRA. The patch available at present improves admin's 
control, improves our switchboard logic, improves maintainability, and deletes 
almost as much code as it adds. 

> Allow admins to disable compile and other commands
> --------------------------------------------------
>
>                 Key: HIVE-5400
>                 URL: https://issues.apache.org/jira/browse/HIVE-5400
>             Project: Hive
>          Issue Type: Sub-task
>            Reporter: Brock Noland
>            Assignee: Edward Capriolo
>         Attachments: HIVE-5400.patch, HIVE-5400.patch, HIVE-5400.patch
>
>
> From here: 
> https://issues.apache.org/jira/browse/HIVE-5253?focusedCommentId=13782220&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13782220
>  I think we should afford admins who want to disable this functionality the 
> ability to do so. Since such admins might want to disable other commands such 
> as add or dfs, it wouldn't be much trouble to allow them to do this as well. 
> For example we could have a configuration option "hive.available.commands" 
> (or similar) which specified add,set,delete,reset, etc by default. Then check 
> this value in CommandProcessorFactory. It would probably make sense to add 
> this property to the restrict list.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to