InvisibleProgrammer commented on code in PR #6412:
URL: https://github.com/apache/hive/pull/6412#discussion_r3045906906
##########
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java:
##########
@@ -398,6 +407,70 @@ private TGetOperationStatusResp waitForResultSetStatus()
throws SQLException {
return statusResp;
}
+ /**
+ * When {@code SET hive.query.timeout.seconds=...} succeeds, remember the
effective value on the
+ * connection so {@code TIMEDOUT_STATE} can report it if the server omits
{@code errorMessage}
+ * (HIVE-28265).
+ */
+ private void trackSessionQueryTimeoutIfSet(String sql) {
+ if (sql == null) {
+ return;
+ }
+ Matcher m = SET_HIVE_QUERY_TIMEOUT_SECONDS.matcher(sql);
+ Long lastSec = null;
+ while (m.find()) {
Review Comment:
I found this PR interesting.
Unfortunately, I have no time to finish this review as I go for a long
vacation.
But this part made me suspicious as I'm pretty sure we usually don't read
Hive this way.
So, I read the Jira ticket itself:
https://issues.apache.org/jira/browse/HIVE-28265
My fault, I should start with this one.
Actually, the ticket says the feature itself is already working but we get a
wrong error message.
Just thinking out loud:
As I see HiveStatement doesn't contain any reference to Hive Configuration.
Creating a hiveConf object is not a top of my mind but I affraid with this way
you ignore the actual HiveConf loaded in the Hive Server session. I'm sad that
I have no time to debug it out but for me, it looks suspicious.
I bet Hive already has it's method to read the SET ... commands out. As you
can see, we have no such (or similar) parsing logic to read the Hive settings
but still, if you run a set command, you can easily read the value from
HiveConf. It would worth doing a debug session and figuring out how Hive
exactly handles session level configurations.
Based on that I would say maybe there is a place where
HiveStatement.setQueryTimeout should be called but it is not.
Anyway, good luck with the PR. If you have still have open questions at the
end of the next week, I would be happy to help and learn this part of the code.
--
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]
