You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "difin (via GitHub)" <gi...@apache.org> on 2024/10/28 16:58:54 UTC
[PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
difin opened a new pull request, #5520:
URL: https://github.com/apache/hive/pull/5520
…d-users
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
1. Change `'hive.server2.logging.operation.level'` to 'NONE' by default to hide exception stack traces from end-users.
2. Add query id to HiveSQLException's message when the operational log is disabled or when its level = 'NONE' or when `'hive.server2.async.exec.async.compile' `= 'true', because in all these cases the operational log is not displayed and therefore a query id is also not displayed. Using query id more info can easily be retrieved from Hiveserver2 log.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
3. If you fix a bug, you can clarify why it is a bug.
-->
**Background**:
Hive operational log is a query log that contains messages for a specific query id and is intended for Hive clients.
When operational log is enabled, it shows query id, exception stack traces and other messages in Hue and Beeline (see screenshot attached). Hive clients fetch the operational log using Hive-specific jdbc method `HiveStatement.getQueryLog`.
The following settings control operational log behavior:
`Hive.server2.logging.operation.enabled`: true/false (true by default)
`hive.server2.logging.operation.level`:
NONE: Ignore any logging
EXECUTION: Log completion of tasks
PERFORMANCE: Execution + Performance logs
VERBOSE: All logs
Log level can be changed on session level.
The default log level is EXECUTION, which displays exception stack traces which is not good as a default for non-technical users.
**Problem Description**:
Currently, when `'hive.server2.async.exec.async.compile' = 'true' `and an exception happens, an operational log is not displayed in Beeline and Hue (because stmtHandle is empty) and only exception message is displayed. Query id is not displayed.
When `'hive.server2.async.exec.async.compile' = 'false'` and operational log level != 'NONE', operational log is displayed with query id and exception stack trace and also exception message is displayed.
End-users do not need to see exception stack traces by default, but when a query fails they should know Query ID, because using one they can find stack traces and other info in HiveServer2 log.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### Is the change a dependency upgrade?
<!--
If yes, please attach a file with output from mvn dependency:tree to validate a complete upgrade of dependency.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Added new JDBC and Beeline integration tests.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2447047984
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820361629
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -880,6 +881,12 @@ private boolean connectUsingArgs(BeelineParser beelineParser, CommandLine cl) {
setHiveConfVar(key, hiveConfs.getProperty(key));
}
+ String opLogLevelVarName = HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL.varname;
Review Comment:
i think that should be downstream only change where could disable `HIVE_SERVER2_LOGGING_OPERATION_LEVEL`
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823239643
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
@difin, are we disabling it for the JDBC connections?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823270389
##########
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java:
##########
@@ -211,13 +210,13 @@ private void prepare(QueryState queryState) throws HiveSQLException {
setHasResultSet(driver.hasResultSet());
} catch (CommandProcessorException e) {
setState(OperationState.ERROR);
- throw toSQLException("Error while compiling statement", e);
+ throw toSQLException("Error while compiling statement", e, queryState);
Review Comment:
don't we add the queryId based on operationLog mode?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823270389
##########
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java:
##########
@@ -211,13 +210,13 @@ private void prepare(QueryState queryState) throws HiveSQLException {
setHasResultSet(driver.hasResultSet());
} catch (CommandProcessorException e) {
setState(OperationState.ERROR);
- throw toSQLException("Error while compiling statement", e);
+ throw toSQLException("Error while compiling statement", e, queryState);
Review Comment:
don't we add the queryId based on operationLog mode?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823239643
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
@difin, are we disabling it for the JDBC connections by default?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage and set queryId in class property in constructor
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%s; Query ID: %s", errorMsg, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823239643
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
are we disabling it for the JDBC connections?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2448600377
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823276267
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -880,6 +881,11 @@ private boolean connectUsingArgs(BeelineParser beelineParser, CommandLine cl) {
setHiveConfVar(key, hiveConfs.getProperty(key));
}
+ String opLogLevelVarName = HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL.varname;
+ if (getOpts().getHiveConfVariables().get(opLogLevelVarName) == null) {
Review Comment:
can we use `contains`?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage and set queryId in class property in constructor
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%1$s; %2$s: %3$s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage and set queryId as class property in constructor
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%s; Query ID: %s", errorMsg, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820354373
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -58,6 +60,10 @@ public HiveSQLException(String reason) {
super(reason);
}
+ public HiveSQLException(String reason, QueryState queryState) {
+ super(ServiceUtils.addQueryId(reason, queryState), reason);
Review Comment:
i think ServiceUtils should provide `getQueryId` method and final message could be constructed in HiveSQLException
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820347948
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
wait, why change here, when you override on client side?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823287318
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +189,19 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String addQueryId(String message, QueryState queryState) {
+ boolean opLogEnabled = HiveConf.getBoolVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED, false);
+ String opLogLevel = HiveConf.getVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL, StringUtils.EMPTY);
+ boolean asyncCompile = HiveConf.getBoolVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_ASYNC_EXEC_ASYNC_COMPILE, false);
+
+ if ((!opLogEnabled || opLogLevel.equals(OperationLog.LoggingLevel.NONE.name()) || !asyncCompile) &&
Review Comment:
1. what is with `asyncCompile`? we return the operation log regardless if it's enabled?
2. why are we checking if the message doesn't contain QUERY_ID pattern? What if some exceptions do and we still want to get the operation log?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820873256
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -880,6 +881,12 @@ private boolean connectUsingArgs(BeelineParser beelineParser, CommandLine cl) {
setHiveConfVar(key, hiveConfs.getProperty(key));
}
+ String opLogLevelVarName = HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL.varname;
Review Comment:
This is related to the previous comment.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2445310601
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823248022
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +189,19 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String addQueryId(String message, QueryState queryState) {
+ boolean opLogEnabled = HiveConf.getBoolVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED, false);
+ String opLogLevel = HiveConf.getVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL, StringUtils.EMPTY);
+ boolean asyncCompile = HiveConf.getBoolVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_ASYNC_EXEC_ASYNC_COMPILE, false);
+
+ if ((!opLogEnabled || opLogLevel.equals(OperationLog.LoggingLevel.NONE.name()) || !asyncCompile) &&
Review Comment:
why wrap in brackets OR predicates?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%1$s; %2$s: %3$s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823274425
##########
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java:
##########
@@ -574,7 +573,9 @@ private AbstractSerDe getSerDe() throws SQLException {
serde.initialize(queryState.getConf(), props, null);
} catch (Exception ex) {
- throw new SQLException("Could not create ResultSet: " + ex.getMessage(), ex);
+ throw new SQLException(
+ String.format("Could not create ResultSet: %1$s; %2$s: %3$s",
Review Comment:
why is this change?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2453086260
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820343983
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -182,6 +182,7 @@ public class BeeLine implements Closeable {
private static final String HIVE_CONF_PREFIX = "--hiveconf";
private static final String PROP_FILE_PREFIX = "--property-file";
public static final String PASSWD_MASK = "[passwd stripped]";
+ public static final String BEELINE_DEFAULT_OPERATIONAL_LOG_LEVEL = "EXECUTION";
Review Comment:
keep it simple: DEFAULT_OPERATIONAL_LOG_LEVEL, it's inside of BeeLine class
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -182,6 +182,7 @@ public class BeeLine implements Closeable {
private static final String HIVE_CONF_PREFIX = "--hiveconf";
private static final String PROP_FILE_PREFIX = "--property-file";
public static final String PASSWD_MASK = "[passwd stripped]";
+ public static final String BEELINE_DEFAULT_OPERATIONAL_LOG_LEVEL = "EXECUTION";
Review Comment:
keep it simple: `DEFAULT_OPERATIONAL_LOG_LEVEL`, it's inside of BeeLine class
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2442352460
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2452104545
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820979989
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -182,6 +182,7 @@ public class BeeLine implements Closeable {
private static final String HIVE_CONF_PREFIX = "--hiveconf";
private static final String PROP_FILE_PREFIX = "--property-file";
public static final String PASSWD_MASK = "[passwd stripped]";
+ public static final String BEELINE_DEFAULT_OPERATIONAL_LOG_LEVEL = "EXECUTION";
Review Comment:
Done
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -58,6 +60,10 @@ public HiveSQLException(String reason) {
super(reason);
}
+ public HiveSQLException(String reason, QueryState queryState) {
+ super(ServiceUtils.addQueryId(reason, queryState), reason);
Review Comment:
Done
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823243595
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +189,19 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String addQueryId(String message, QueryState queryState) {
Review Comment:
maybe `getErrorMsg(QueryState queryState, String reason)`
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826565268
##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -182,6 +182,7 @@ public class BeeLine implements Closeable {
private static final String HIVE_CONF_PREFIX = "--hiveconf";
private static final String PROP_FILE_PREFIX = "--property-file";
public static final String PASSWD_MASK = "[passwd stripped]";
+ public static final String DEFAULT_OPERATIONAL_LOG_LEVEL = "EXECUTION";
Review Comment:
let's drop it
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin merged PR #5520:
URL: https://github.com/apache/hive/pull/5520
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823248022
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +189,19 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String addQueryId(String message, QueryState queryState) {
+ boolean opLogEnabled = HiveConf.getBoolVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED, false);
+ String opLogLevel = HiveConf.getVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL, StringUtils.EMPTY);
+ boolean asyncCompile = HiveConf.getBoolVar(queryState.getConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_ASYNC_EXEC_ASYNC_COMPILE, false);
+
+ if ((!opLogEnabled || opLogLevel.equals(OperationLog.LoggingLevel.NONE.name()) || !asyncCompile) &&
Review Comment:
why wrap in brackets OR predicates?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%1$s; %2$s: %3$s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1823368668
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +189,19 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String addQueryId(String message, QueryState queryState) {
Review Comment:
Done
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2444713256
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2445640826
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820871906
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
I overrided only in Beeline because as we said Beeline is mostly used by developers.
It's for other Hive client tools, like Hue.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2469032782
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [16 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2442974755
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1830458552
##########
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestExceptionMessageWithJdbc.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hive.jdbc;
+
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hadoop.hive.conf.HiveConfForTest;
+import org.apache.hadoop.hive.ql.ServiceContext;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestExceptionMessageWithJdbc {
+ private static MiniHS2 miniHS2;
+ private static Connection conDefault;
+ private static Connection conTestDb;
+ private static final String TEST_DB_NAME = "test_logging_level";
+ private static final String USERNAME = System.getProperty("user.name");
+ private static final String PASSWORD = "bar";
+
+ @BeforeClass
+ public static void setupBeforeClass() throws Exception {
+ MiniHS2.cleanupLocalDir();
+ HiveConf conf = getNewHiveConf();
+ try {
+ startMiniHS2(conf);
+ } catch (Exception e) {
+ System.out.println("Unable to start MiniHS2: " + e);
+ throw e;
+ }
+
+ try {
+ openDefaultConnections();
+ } catch (Exception e) {
+ System.out.println("Unable to open default connections to MiniHS2: " + e);
+ throw e;
+ }
+ Statement stmt = conDefault.createStatement();
+ stmt.execute("drop database if exists " + TEST_DB_NAME + " cascade");
+ stmt.execute("create database " + TEST_DB_NAME);
+ stmt.close();
+
+ try {
+ openTestConnections();
+ } catch (Exception e) {
+ System.out.println("Unable to open default connections to MiniHS2: " + e);
+ throw e;
+ }
+ }
+
+ private static Connection getConnection() throws Exception {
+ return getConnection(miniHS2.getJdbcURL(), USERNAME, PASSWORD);
+ }
+
+ private static Connection getConnection(String dbName) throws Exception {
+ return getConnection(miniHS2.getJdbcURL(dbName), USERNAME, PASSWORD);
+ }
+
+ private static Connection getConnection(String jdbcURL, String user, String pwd)
+ throws SQLException {
+ Connection conn = DriverManager.getConnection(jdbcURL, user, pwd);
+ assertNotNull(conn);
+ return conn;
+ }
+
+ @AfterClass
+ public static void tearDownAfterClass() throws Exception {
+ Statement stmt = conDefault.createStatement();
+ stmt.execute("set hive.support.concurrency = false");
+ stmt.execute("drop database if exists " + TEST_DB_NAME + " cascade");
+ stmt.close();
+ if (conTestDb != null) {
+ conTestDb.close();
+ }
+ if (conDefault != null) {
+ conDefault.close();
+ }
+ stopMiniHS2();
+ cleanupMiniHS2();
+ }
+
+ private static void startMiniHS2(HiveConf conf) throws Exception {
+ MiniHS2.Builder builder = new MiniHS2.Builder().withConf(conf).cleanupLocalDirOnStartup(false);
+ miniHS2 = builder.build();
+
+ Map<String, String> confOverlay = new HashMap<>();
+ miniHS2.start(confOverlay);
+ }
+
+ private static void stopMiniHS2() {
+ if ((miniHS2 != null) && (miniHS2.isStarted())) {
+ miniHS2.stop();
+ }
+ }
+
+ private static void cleanupMiniHS2() throws IOException {
+ if (miniHS2 != null) {
+ miniHS2.cleanup();
+ }
+ MiniHS2.cleanupLocalDir();
+ }
+
+ private static void openDefaultConnections() throws Exception {
+ conDefault = getConnection();
+ }
+
+ private static void openTestConnections() throws Exception {
+ conTestDb = getConnection(TEST_DB_NAME);
+ }
+
+ private static HiveConf getNewHiveConf() {
+ HiveConf conf = new HiveConfForTest(TestExceptionMessageWithJdbc.class);
+ conf.setVar(ConfVars.HIVE_EXECUTION_ENGINE, "mr");
Review Comment:
Why are we running the tests with MR, MR is deprecated we should avoid adding new tests with it
##########
ql/src/java/org/apache/hadoop/hive/ql/ServiceContext.java:
##########
@@ -30,6 +30,7 @@ public class ServiceContext {
private final String clusterId;
private final Supplier<String> host;
private final Supplier<Integer> port;
+ public static final String QUERY_ID = "Query ID";
Review Comment:
I am not sure if this is the correct place to keep this variable, The Javadoc of ``ServiceContext`` itself says ``Information context about the service, most probably HiveServer2.``
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +191,13 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ @Override
+ public String getMessage(){
+ String errorMsg = super.getMessage() == null ? "Error" : super.getMessage();
+ if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
+ return String.format("%s; Query ID: %s", errorMsg, queryId);
Review Comment:
Shouldn't we check ``queryId`` not null as well? else someone having an old jar which creates this HiveSqlException without QueryId will start getting this with null?
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -70,6 +72,11 @@ public HiveSQLException(Throwable cause) {
super(cause);
}
+ public HiveSQLException(Throwable cause, String queryId) {
Review Comment:
We should even deprecate the old constructors without queryId & remove in the subsequent release
##########
service/src/java/org/apache/hive/service/ServiceUtils.java:
##########
@@ -72,5 +72,4 @@ public static boolean canProvideProgressLog(HiveConf hiveConf) {
return "tez".equals(hiveConf.getVar(HiveConf.ConfVars.HIVE_EXECUTION_ENGINE)) && hiveConf
.getBoolVar(HiveConf.ConfVars.HIVE_SERVER2_INPLACE_PROGRESS);
}
-
Review Comment:
nit
avoid
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +191,13 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ @Override
+ public String getMessage(){
+ String errorMsg = super.getMessage() == null ? "Error" : super.getMessage();
+ if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
Review Comment:
curious: is the contains to avoid appending queryId multiple times, there could be cases where query id is already appended & we want to avoid appending it again?
Something like that?
##########
service/src/java/org/apache/hive/service/cli/operation/Operation.java:
##########
@@ -367,13 +367,13 @@ protected void validateFetchOrientation(FetchOrientation orientation,
EnumSet<FetchOrientation> supportedOrientations) throws HiveSQLException {
if (!supportedOrientations.contains(orientation)) {
throw new HiveSQLException("The fetch type " + orientation.toString() +
- " is not supported for this resultset", "HY106");
+ " is not supported for this resultset", "HY106", queryState.getQueryId());
}
}
- protected HiveSQLException toSQLException(String prefix, CommandProcessorException e) {
+ protected HiveSQLException toSQLException(String prefix, CommandProcessorException e, QueryState queryState) {
Review Comment:
Why are you having ``queryState`` as a parameter & then extracting ``queryId`` from it, we can directly have ``queryId`` as a parameter? as in case of constructor of ``HiveSQLException``
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2458424108
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5520:
URL: https://github.com/apache/hive/pull/5520#issuecomment-2460845177
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520) **Quality Gate passed**
Issues
 [15 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5520&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5520&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5520&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5520)
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage and set queryId in class property in constructor
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%1$s; %2$s: %3$s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage and set queryId in class property in constructor
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%1$s; %2$s: %3$s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage and set queryId in class property in constructor
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%s; %s: %s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Add query id to HiveSQLException [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1826563864
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -150,4 +185,11 @@ public static TStatus toTStatus(Exception e) {
return tStatus;
}
+ private static String getErrorMsg(String message, String queryId) {
Review Comment:
i think it could be done simpler, just override the Throwable#getMessage
````
@Override
String getMessage(){
String errorMsg = super.getMessage();
if (!errorMsg.contains(ServiceContext.QUERY_ID)) {
return String.format("%1$s; %2$s: %3$s", errorMsg, ServiceContext.QUERY_ID, queryId);
} else {
return errorMsg;
}
}
````
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820354373
##########
service/src/java/org/apache/hive/service/cli/HiveSQLException.java:
##########
@@ -58,6 +60,10 @@ public HiveSQLException(String reason) {
super(reason);
}
+ public HiveSQLException(String reason, QueryState queryState) {
+ super(ServiceUtils.addQueryId(reason, queryState), reason);
Review Comment:
i think you can extract `queryId` from the `queryState` here and prepare final message in HiveSQLException constructor
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820871906
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
We override only on Beeline because as we said Beeline is mostly used by developers.
It's for other Hive client tools, like Hue.
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
We override only in Beeline because as we said Beeline is mostly used by developers.
It's for other Hive client tools, like Hue.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28579: Prevent exception stack traces to be displayed to Hive en… [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5520:
URL: https://github.com/apache/hive/pull/5520#discussion_r1820871906
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -4117,7 +4117,7 @@ public static enum ConfVars {
"${system:java.io.tmpdir}" + File.separator + "${system:user.name}" + File.separator +
"operation_logs",
"Top level directory where operation logs are stored if logging functionality is enabled"),
- HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "EXECUTION",
+ HIVE_SERVER2_LOGGING_OPERATION_LEVEL("hive.server2.logging.operation.level", "NONE",
Review Comment:
I overrided only in Beeline because as we said Beeline is mostly used by developers.
The change in HiveConf is for other Hive client tools, like Hue.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org