[
https://issues.apache.org/jira/browse/HIVE-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13695920#comment-13695920
]
Phabricator commented on HIVE-4569:
-----------------------------------
cwsteinbach has commented on the revision "HIVE-4569 [jira] GetQueryPlan api in
Hive Server2".
INLINE COMMENTS
ql/src/java/org/apache/hadoop/hive/ql/TaskStatus.java:1 Missing ASF license
header.
ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java:1019 This looks
like a debug statement. Should it be removed?
ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java:95 Can you add some
comments here explaining what each one of these states actually means? Also, do
we need an UNKNOWN state? I included one in the Thrift IDL OperationState, but
in retrospect that was probably a mistake.
service/if/TCLIService.thrift:34 As discussed earlier we shouldn't add this
dependency to the HS2 API. Please remove it and return the Task information in
JSON or XML.
service/if/TCLIService.thrift:41 We need to bump the version number since
this patch extends the HS2 API with new functionality. Can you also please add
a comment here briefly summarize what was added in the new version?
service/if/TCLIService.thrift:594 Thrift allows you specify default values
for optional fields. I think we should set this value to 'false' by default.
service/if/TCLIService.thrift:866 Just want to double-check that TTaskState
and TTaskStatus will be removed since the plan state will be serialized as JSON
or XML, right?
service/if/TCLIService.thrift:1003 Where is TGetQueryPlanReq? The comments at
the top stipulate that every RPC has it's own req/resp message pair.
service/if/TCLIService.thrift:1006 Just double-checking that this will be
changed to a string.
service/if/TCLIService.thrift:1043 Please don't overload TExecuteStatementReq.
service/src/java/org/apache/hive/service/cli/CLIService.java:149 Thrift makes
it easy to add additional optional parameters without breaking backward
compatibility, but not Java. I'd recommend creating a new executeStatementAsync
call to ICLIService (and here) instead of modifying the method signature. Also,
that probably indicates that we should add a new complimentary RPC to the HS2
Thrift IDL instead of using adding an optional parameter to ExecuteStatement
just to keep these things in sync.
service/src/java/org/apache/hive/service/cli/CLIService.java:318
s/:getQueryPlan/: getQueryPlan/
ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java:367 I don't think this
method is thread-safe. I recommend replacing the four boolean state variables
(started, initialized, isdone, queued, wth??) with the single TaskState enum
you added and make sure that all access to this state variable is synchronized.
REVISION DETAIL
https://reviews.facebook.net/D11469
To: JIRA, jaideepdhok
Cc: cwsteinbach
> GetQueryPlan api in Hive Server2
> --------------------------------
>
> Key: HIVE-4569
> URL: https://issues.apache.org/jira/browse/HIVE-4569
> Project: Hive
> Issue Type: Bug
> Components: HiveServer2
> Reporter: Amareshwari Sriramadasu
> Assignee: Jaideep Dhok
> Attachments: git-4569.patch, HIVE-4569.D10887.1.patch,
> HIVE-4569.D11469.1.patch
>
>
> It would nice to have GetQueryPlan as thrift api. I do not see GetQueryPlan
> api available in HiveServer2, though the wiki
> https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Thrift+API
> contains, not sure why it was not added.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
