> On Nov. 8, 2013, 8:33 p.m., Prasad Mujumdar wrote: > > @Vaibhav, thanks for taking the issue forward and putting a new patch! > > > > I do have a high level comment on the approach. The 'status' returned by > > HS2 RPC is suppose to be the status of that particular API's execution. > > Where as in this case, we are overloading the 'status' field to return the > > status of a different (ie. ExecuteStatement()) status. > > For example, if you call GetStatus() with a non-existing operation id then > > you would get an error status. This error is for failure of the GetStatus() > > itself. On the other hand if you call GetStatus() for an async query that > > failed, then you will also get the error status. However this error is not > > for the current GetStatus() operation, but for the last ExecuteStatement() > > operation. > > The current implementation of the JDBC driver (or CLIClient in general) > > will work with this, but perhaps its not a clean way to implement it. Have > > you considered adding a new field in the GetStatus response to return the > > error status of the actual execute operation ? > > > >
Thanks for pointing out the design intent Prasad. I agree, expanding the API to include the operation error in GetStatus response is a cleaner implementation. I've made the relevant changes. Let me know if you have any feedback on the latest patch. Thanks! - Vaibhav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15151/#review28572 ----------------------------------------------------------- On Nov. 11, 2013, 7:23 p.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15151/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2013, 7:23 p.m.) > > > Review request for hive, Prasad Mujumdar and Thejas Nair. > > > Bugs: HIVE-5230 > https://issues.apache.org/jira/browse/HIVE-5230 > > > Repository: hive-git > > > Description > ------- > > [HIVE-4617|https://issues.apache.org/jira/browse/HIVE-4617] provides support > for async execution in HS2. When a background thread gets an error, currently > the client can only poll for the operation state and also the error with its > stacktrace is logged. However, it will be useful to provide a richer error > response like thrift API does with TStatus (which is constructed while > building a Thrift response object). > > > Diffs > ----- > > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/CLIServiceTest.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceTest.java > PRE-CREATION > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java fce19bf > service/if/TCLIService.thrift 1f49445 > service/src/java/org/apache/hive/service/cli/CLIService.java 8c85386 > service/src/java/org/apache/hive/service/cli/CLIServiceClient.java 14ef54f > service/src/java/org/apache/hive/service/cli/EmbeddedCLIServiceClient.java > 9dca874 > service/src/java/org/apache/hive/service/cli/HiveSQLException.java 74e8b94 > service/src/java/org/apache/hive/service/cli/ICLIService.java f647ce6 > service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 > service/src/java/org/apache/hive/service/cli/OperationStatus.java > PRE-CREATION > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 6f4b8dc > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > bcdb67f > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > 4ee1b74 > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > 9df110e > > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java > 9bb2a0f > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java cd9d99a > > service/src/test/org/apache/hive/service/cli/thrift/ThriftCLIServiceTest.java > ff7166d > > Diff: https://reviews.apache.org/r/15151/diff/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
