jfsii commented on code in PR #4137:
URL: https://github.com/apache/hive/pull/4137#discussion_r1147867743


##########
service/src/java/org/apache/hive/service/cli/thrift/RetryingThriftCLIServiceClient.java:
##########
@@ -310,7 +310,7 @@ protected synchronized TTransport connect(HiveConf conf) 
throws HiveSQLException
 
     String host = conf.getVar(HiveConf.ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST);
     int port = conf.getIntVar(HiveConf.ConfVars.HIVE_SERVER2_THRIFT_PORT);
-    int maxThriftMessageSize = (int) 
conf.getSizeVar(HiveConf.ConfVars.HIVE_THRIFT_CLIENT_MAX_MESSAGE_SIZE);
+    int maxThriftMessageSize = (int) 
Math.min(conf.getSizeVar(HiveConf.ConfVars.HIVE_THRIFT_CLIENT_MAX_MESSAGE_SIZE),Integer.MAX_VALUE);

Review Comment:
   I'm not sure the sizeValidator gets executed when a hive-site.xml is loaded 
(only when a SET is called), I'm not 100% sure though. That is the reason for 
my paranoia. Also having a helper method for that does the min and max range 
would make it usable in other instances in which we need to convert a "size" to 
an int or some other range.
   It is not an absolute requirement, mostly a suggestion - if 
@saihemanth-cloudera is okay with the state of the patch that is good enough 
with me. (Functionally it is correct which is what matters).



-- 
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]

Reply via email to