HIVE-26345: SQLOperation class output real exception message to jdbc client

[zhangbutao]

What changes were proposed in this pull request?

Why are the changes needed?

See details from https://issues.apache.org/jira/browse/HIVE-26345 SQLOperation class output real exception message to jdbc client and user can amend the query based on real exception.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Local cluster test.

Before the PR, beeline dispaly exception: Error: Error running query (state=,code=0)

With the PR, beeline dispaly exception: Error: Error running query: java.lang.RuntimeException: To use DbTxnManager you must set hive.support.concurrency=true (state=,code=0)

[zhangbutao]

@belugabehr @pvary @zabetak wdyt? Thank you.

[zhangbutao]

As I wrote under #3364 this message concatenation pattern has trade-offs (between user and developper).

Have you considered attacking this from the beeline side?

@zabetak Thank you for your detailed explanation! I will do some further research based on your suggestion.

[zhangbutao]

@zabetak I checked beeline code adn hs2 code, and i still think this change should be fixed in hs2 side. Here's my test, please correct me if i am wrong. Thank you.

1. Use following query to test exception message: set hive.support.concurrency=false; set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager; create table testacid(id int) stored as orc tblproperties('transactional'='true');

2. The final result which wraps exception returned from HS2 side, a TExecuteStatementResp object https://github.com/apache/hive/blob/8f1a5b6854daf5fb4814632f9356ef5fe7bb6964/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java#L672-L679 And the final exception massage is set using tStatus.setErrorMessage(getMessage());, that is say client can only get simplified exception message(Error running query) instead of full exception stacktrace: https://github.com/apache/hive/blob/8f1a5b6854daf5fb4814632f9356ef5fe7bb6964/service/src/java/org/apache/hive/service/cli/HiveSQLException.java#L131

3. The excepton displayed from the Beeline side: Beeline client gets query response (TExecuteStatementResp object) from HS2 side, and uses Utils.verifySuccessWithInfo(execResp.getStatus()) to check status or throw exception: https://github.com/apache/hive/blob/8f1a5b6854daf5fb4814632f9356ef5fe7bb6964/jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java#L353-L354 https://github.com/apache/hive/blob/8f1a5b6854daf5fb4814632f9356ef5fe7bb6964/jdbc/src/java/org/apache/hive/jdbc/Utils.java#L378-L383 The final exception message displayed to client is status.getErrorMessage(), that is "Error running query" returned from HS2 side: https://github.com/apache/hive/blob/8f1a5b6854daf5fb4814632f9356ef5fe7bb6964/service/src/java/org/apache/hive/service/cli/HiveSQLException.java#L117-L118

So, i think the root cause was that HS2 side did not return full valid exception message, and because we often use e.getMessage() to retrieve meaningful exception, we should add message content when throw a exception, e.g.: throw new HiveSQLException("Error running query: " + e.toString(), e);

[zabetak]

Thanks for your detailed analysis @zhangbutao and apologies for the delay. I guess you are right that we should handle the problem in the HS2 side but I still believe that concatenating exception messages is not the right way to move forward.

If we need to inform the client (e.g., beeline) about a specific error maybe we need to use an entry from ErrorMsg class, add a new one, and or do something similar to what happens in Compiler.handleException.

[zhangbutao]

@zabetak After further investigation, i found HIVE-22526 separateed txn related codes from compile codes which lead to exception(errorCode 10264) from txn not being catched and converted into CommandProcessorException. I fixed this regression by catch txn exception in txn code block.

After fixing, we can get txn exception whth errorCode(10264) and state(42000). Error: Error while compiling statement: FAILED: RuntimeException [Error 10264]: To use DbTxnManager you must set hive.support.concurrency=true (state=42000,code=10264)

BTW, for other exception which may not exist in ErrorMsg class, shoud we add a generic error code in ErrorMsg class or simply throw message? I have seen lots of code which handle HiveSQLException like this: throw new HiveSQLException("Error running query: " + e.toString(), e);

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Feel free to reach out on the [email protected] list if the patch is in need of reviews.