[GitHub] [hive] SourabhBadhya commented on a diff in pull request #4110: HIVE-27133: Round off limit value greater than int_max to int_max

["SourabhBadhya (via GitHub)" <gi...@apache.org>]

SourabhBadhya commented on code in PR #4110:
URL: https://github.com/apache/hive/pull/4110#discussion_r1137062590


##########
ql/src/test/queries/clientpositive/limit_max_int.q:
##########
@@ -0,0 +1,6 @@
+--! qt:dataset:src
+select key from src limit 214748364700;
+select key from src where key = '238' limit 214748364700;
+select * from src where key = '238' limit 214748364700;
+select src.key, count(src.value) from src group by src.key limit 214748364700;
+select * from ( select key from src limit 3) sq1 limit 214748364700;

Review Comment:
   nit: Please add a newline at the end of the qfile.



##########
common/src/java/org/apache/hive/common/util/HiveStringUtils.java:
##########
@@ -1174,4 +1175,25 @@ private static boolean isComment(String line) {
     return lineTrimmed.startsWith("#") || lineTrimmed.startsWith("--");
   }
 
+  /**
+   * Returns integer value of a string. If the string value exceeds max int, returns Integer.MAX_VALUE
+   * else if the string value is less than min int, returns Integer.MIN_VALUE
+   *
+   * @param value value of the input string
+   * @return integer
+   */
+  public static int convertStringToBoundedInt(String value) {
+    try {
+      BigInteger bigIntValue = new BigInteger(value);
+      if (bigIntValue.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) > 0) {
+        return Integer.MAX_VALUE;

Review Comment:
   @vamshikolanu 
   I agree with @jfsii. Converting a large number to Integer.MAX_VALUE is misleading the user. 
   Consider the following query - 
   `INSERT INTO TABLE destinationTable SELECT * FROM sourceTable LIMIT <some_large_number>;`
   The insert will write records based on the output of the SELECT operator. In this case, since we have converted it to Integer.MAX_VALUE, the number of records written will be equal to Integer.MAX_VALUE which might not be what the user wants.
   
   Perhaps adding a meaningful exception is better. In the long term, adding support for large integers for LIMIT clauses is even more better.



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