kasakrisz commented on code in PR #5637:
URL: https://github.com/apache/hive/pull/5637#discussion_r2166619422


##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java:
##########
@@ -1166,40 +1169,42 @@ private long evaluateComparator(Statistics stats, 
AnnotateStatsProcCtx aspCtx, E
                 return Math.round(((double) (maxValue - value) / (maxValue - 
minValue)) * currNumRows);
               }
             }
-          } else if (colTypeLowerCase.equals(serdeConstants.BIGINT_TYPE_NAME) 
||
-              colTypeLowerCase.equals(serdeConstants.TIMESTAMP_TYPE_NAME)) {
-            long value;
-            if (colTypeLowerCase.equals(serdeConstants.TIMESTAMP_TYPE_NAME)) {
-              TimestampWritableV2 timestampWritable = new 
TimestampWritableV2(Timestamp.valueOf(boundValue));
-              value = timestampWritable.getTimestamp().toEpochSecond();
-            } else {
-              value = Long.parseLong(boundValue);
-            }
-            long maxValue = cs.getRange().maxValue.longValue();
-            long minValue = cs.getRange().minValue.longValue();
+          } else if 
(colTypeLowerCase.startsWith(serdeConstants.DECIMAL_TYPE_NAME) ||
+              colTypeLowerCase.equals(serdeConstants.BIGINT_TYPE_NAME)) {
+            BigDecimal value = new BigDecimal(boundValue);
+            BigDecimal maxValue = new 
BigDecimal(cs.getRange().maxValue.toString());
+            BigDecimal minValue = new 
BigDecimal(cs.getRange().minValue.toString());
+            int minComparison = value.compareTo(minValue);
+            int maxComparison = value.compareTo(maxValue);

Review Comment:
   You mean this one?
   ```
   Math.round(((double) (value - minValue) / (maxValue - minValue)) * 
currNumRows)
   ```
   It seems that it is converted to double. Maybe the overflow is caused 
because the toDouble conversion is happening too late.
   
   I think this could be better to avoid overflow
   ```
   Math.round((((double)value - minValue) / ((double) maxValue - minValue)) * 
currNumRows)
   ```
   
   `explainanalyze_5.q` doesn't seems to cover this scenario. Are there any 
other tests that covers?



-- 
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-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to