zabetak commented on code in PR #3364:
URL: https://github.com/apache/hive/pull/3364#discussion_r902384538
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcSerDe.java:
##########
@@ -131,7 +131,8 @@ public void initialize(Configuration configuration,
Properties tableProperties,
row = new ArrayList<>(hiveColumnNames.length);
}
} catch (Exception e) {
- throw new SerDeException("Caught exception while initializing the
SqlSerDe", e);
+ log.error("Caught exception while initializing the SqlSerDe", e);
+ throw new SerDeException(e);
Review Comment:
It seems that this is a problem of the client (Beeline) and not a problem of
the server (HS2).
A stacktrace can be composed of many nested exceptions one causing the
other. It doesn't seem like a very good idea to me to just concatenate the
messages when propagating the exception from one abstraction to the other.
```java
throw new SerDeException("Caught exception while initializing the SqlSerDe:
" + e.getMessage(), e);
```
The snippet above will make the exception in the HS2 logs more verbose than
necessary since it will essentially have duplicate information.
Even the current version of the code contains duplicate information:
```java
throw new SerDeException("Caught exception while initializing the SqlSerDe",
e);
```
The stacktrace shows that this exception is thrown from inside a `catch`
block so mentioning "Caught exception while" does not add additional info but
just states the obvious. Moreover, the stack trace also shows that we are
inside the initialize method, in `JdbcSerDe` class so putting "initializing the
SqlSerDe" is redundant and possibly misleading cause there is no `SqlSerDe`
class.
What I want to highlight is that exceptions are for developers and not for
end users. It is very important to provide to the end user meaningful error
messages but we should be mindful on how we achieve this result.
--
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]
