[
https://issues.apache.org/jira/browse/HIVE-24333?focusedWorklogId=509800&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-509800
]
ASF GitHub Bot logged work on HIVE-24333:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 10/Nov/20 16:22
Start Date: 10/Nov/20 16:22
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1629:
URL: https://github.com/apache/hive/pull/1629#discussion_r520693419
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/Driver.java
##########
@@ -149,207 +149,64 @@ private CommandProcessorResponse run(String command,
boolean alreadyCompiled) th
runInternal(command, alreadyCompiled);
return new CommandProcessorResponse(getSchema(), null);
} catch (CommandProcessorException cpe) {
- SessionState ss = SessionState.get();
- if (ss == null) {
- throw cpe;
- }
- MetaDataFormatter mdf = MetaDataFormatUtils.getFormatter(ss.getConf());
- if (!(mdf instanceof JsonMetaDataFormatter)) {
- throw cpe;
- }
- /*Here we want to encode the error in machine readable way (e.g. JSON)
- * Ideally, errorCode would always be set to a canonical error defined
in ErrorMsg.
- * In practice that is rarely the case, so the messy logic below tries
to tease
- * out canonical error code if it can. Exclude stack trace from output
when
- * the error is a specific/expected one.
- * It's written to stdout for backward compatibility (WebHCat consumes
it).*/
- try {
- if (cpe.getCause() == null) {
- mdf.error(ss.out, cpe.getMessage(), cpe.getResponseCode(),
cpe.getSqlState());
- throw cpe;
- }
- ErrorMsg canonicalErr = ErrorMsg.getErrorMsg(cpe.getResponseCode());
- if (canonicalErr != null && canonicalErr != ErrorMsg.GENERIC_ERROR) {
- /*Some HiveExceptions (e.g. SemanticException) don't set
- canonical ErrorMsg explicitly, but there is logic
- (e.g. #compile()) to find an appropriate canonical error and
- return its code as error code. In this case we want to
- preserve it for downstream code to interpret*/
- mdf.error(ss.out, cpe.getMessage(), cpe.getResponseCode(),
cpe.getSqlState(), null);
- throw cpe;
- }
- if (cpe.getCause() instanceof HiveException) {
- HiveException rc = (HiveException)cpe.getCause();
- mdf.error(ss.out, cpe.getMessage(),
rc.getCanonicalErrorMsg().getErrorCode(), cpe.getSqlState(),
- rc.getCanonicalErrorMsg() == ErrorMsg.GENERIC_ERROR ?
StringUtils.stringifyException(rc) : null);
- } else {
- ErrorMsg canonicalMsg =
ErrorMsg.getErrorMsg(cpe.getCause().getMessage());
- mdf.error(ss.out, cpe.getMessage(), canonicalMsg.getErrorCode(),
cpe.getSqlState(),
- StringUtils.stringifyException(cpe.getCause()));
- }
- } catch (HiveException ex) {
- CONSOLE.printError("Unable to JSON-encode the error",
StringUtils.stringifyException(ex));
- }
+ processRunException(cpe);
throw cpe;
}
}
private void runInternal(String command, boolean alreadyCompiled) throws
CommandProcessorException {
DriverState.setDriverState(driverState);
- driverState.lock();
- try {
- if (driverContext != null && driverContext.getPlan() != null
- && driverContext.getPlan().isPrepareQuery()
- && !driverContext.getPlan().isExplain()) {
- LOG.info("Skip running tasks for prepare plan");
- return;
- }
- if (alreadyCompiled) {
- if (driverState.isCompiled()) {
- driverState.executing();
- } else {
- String errorMessage = "FAILED: Precompiled query has been cancelled
or closed.";
- CONSOLE.printError(errorMessage);
- throw DriverUtils.createProcessorException(driverContext, 12,
errorMessage, null, null);
- }
- } else {
- driverState.compiling();
- }
- } finally {
- driverState.unlock();
+ if (driverContext != null && driverContext.getPlan() != null &&
Review comment:
I do see that the `driverContext` plan is set in a few different places.
I'm not sure if this can be checked outside of a lock. At the very least, I
would minimize the risk a bit:
```
QueryPlan plan = driverContext.getPlan();
if (plan != null
&& plan.isPrepareQuery()
&& !plan.isExplain()) {
```
At least you know that the value of `plan` cannot possibly change between
calls.
----------------------------------------------------------------
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
Issue Time Tracking
-------------------
Worklog Id: (was: 509800)
Time Spent: 40m (was: 0.5h)
> Cut long methods in Driver to smaller, more manageable pieces
> -------------------------------------------------------------
>
> Key: HIVE-24333
> URL: https://issues.apache.org/jira/browse/HIVE-24333
> Project: Hive
> Issue Type: Sub-task
> Components: Hive
> Reporter: Miklos Gergely
> Assignee: Miklos Gergely
> Priority: Major
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Some methods in Driver are too long to be easily understandable. They should
> be cut into pieces to make them easier to understand.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
