[jira] [Work logged] (HIVE-24333) Cut long methods in Driver to smaller, more manageable pieces

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HIVE-24333?focusedWorklogId=509798&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-509798 ]

ASF GitHub Bot logged work on HIVE-24333:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 10/Nov/20 16:17
            Start Date: 10/Nov/20 16:17
    Worklog Time Spent: 10m 
      Work Description: belugabehr commented on a change in pull request #1629:
URL: https://github.com/apache/hive/pull/1629#discussion_r520689424



##########
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:
       This is nice because it has been moved outside the lock scope.  Do note however that driverContext can never be `null`.  It is defined as `final` and set in the constructor.  Please remove this check.




----------------------------------------------------------------
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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 509798)
    Time Spent: 20m  (was: 10m)

> 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: 20m
>  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)