soumyakanti3578 commented on code in PR #6405:
URL: https://github.com/apache/hive/pull/6405#discussion_r3029915016
##########
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java:
##########
@@ -1401,6 +1401,24 @@ public void testPrintMessageAfterExecuteSetHiveConfig()
throws Throwable {
testScriptFile(scriptText, args(), "Should print this message!...",
OutStream.ERR);
}
+ @Test
+ public void testQuit() throws Throwable {
+ String scriptText =
+ "SET hplsql.onerror='seterror';\n" +
+ "INSERT INTO abc VALUES('Tbl1 Not Exists');\n" +
+ "PRINT 'ERRORCODE: ' || ERRORCODE;\n" +
+ "IF ERRORCODE <> 0 THEN\n" +
+ " PRINT 'Error detected. Exiting...';\n" +
+ " .QUIT ERRORCODE;\n" +
+ "END IF;\n" +
+ "INSERT INTO def VALUES('Tbl2 Not Exists');" +
Review Comment:
nit: not required but if you're adding anything to the PR, consider adding a
`\n` for consistency.
PS: nit: Also not required, but sonar is recommending a text block instead
of concatenating strings, and it would certainly improve readablity.
##########
hplsql/src/main/java/org/apache/hive/hplsql/Exec.java:
##########
@@ -1037,11 +1037,13 @@ public void printExceptions() {
} else if (sig.type == Signal.Type.VALIDATION) {
error(((HplValidationException)sig.exception).getCtx(),
sig.exception.getMessage());
} else if (sig.type == Signal.Type.SQLEXCEPTION) {
- console.printError("Unhandled exception in HPL/SQL");
+ console.printError("Unhandled exception in HPL/SQL: " + sig.value);
} else if (sig.type == Signal.Type.UNSUPPORTED_OPERATION) {
console.printError(sig.value == null ? "Unsupported operation" :
sig.value);
} else if (sig.exception != null) {
console.printError("HPL/SQL error: " +
ExceptionUtils.getStackTrace(sig.exception));
+ } else if (sig.type == Signal.Type.LEAVE_PROGRAM) {
+ console.printError("Leaving Program: " + sig.value);
Review Comment:
If we encounter a `LEAVE_PROGRAM`, does it always mean an `ERROR`? From the
added test it looks like `.QUIT` was intentional, and so maybe shouldn't write
to the error stream? Could this impact production code where a customer could
treat error output as failure?
What are your thoughts on this?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
