thomasrebele commented on code in PR #6712:
URL: https://github.com/apache/hive/pull/6712#discussion_r3894782001


##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/FilterSelectivityEstimator.java:
##########
@@ -1015,8 +1019,8 @@ public static double lessThanSelectivity(KllFloatsSketch 
kll, float value) {
    * @throws IllegalArgumentException if leftValue is equal to rightValue
    */
   public static double betweenSelectivity(KllFloatsSketch kll, float 
leftValue, float rightValue) {
-    // column >= leftValue AND column <= rightValue
-    if (rightValue < leftValue) {
+    // is it possible to fulfill the BETWEEN?
+    if (rightValue < leftValue || rightValue < kll.getMinItem() || leftValue > 
kll.getMaxItem()) {

Review Comment:
   Yes:
   * TestFilterSelectivityEstimator.testBetweenSelectivityRightLowerThanMin
   * TestFilterSelectivityEstimator.testBetweenSelectivityLeftHigherThanMax
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityBetweenRightLowerThanMin
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityBetweenRightLowerThanMinWithSearch
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityBetweenLeftHigherThanMax
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityBetweenLeftHigherThanMaxWithSearch



##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestFilterSelectivityEstimator.java:
##########
@@ -272,6 +273,25 @@ public void testIsHistogramAvailableWhenEmptyArray() {
     Assert.assertFalse(isHistogramAvailable(colStatistics));
   }
 
+  /**
+   * Check the KLL resolution.
+   * <p>
+   * The resolution may not be lower than the minimum rank error [HIVE-29365].
+   * </p> */
+  @Test
+  public void testKllResolution() {
+    Assert.assertEquals(MIN_KLL_SELECTIVITY, betweenSelectivity(KLL, 1.20f, 
1.21f), DELTA);
+
+    
doReturn(Collections.singletonList(stats)).when(tableMock).getColStat(Collections.singletonList(0));
+    RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.AND,
+        REX_BUILDER.makeCall(SqlStdOperatorTable.GREATER_THAN, inputRef0, 
literalFloat(1.20f)),
+        REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN, inputRef0, 
literalFloat(1.21f)));
+    filter = simplify(filter);
+    Assert.assertEquals(SqlKind.SEARCH, filter.getKind());
+    FilterSelectivityEstimator estimator = new 
FilterSelectivityEstimator(scan, mq);
+    Assert.assertEquals(MIN_KLL_SELECTIVITY, 
estimator.estimateSelectivity(filter), DELTA);
+  }

Review Comment:
   It is one layer above the `betweenSelectivity`. It checks whether the 
FilterSelectivityEstimator#estimateSelectivity behaves correctly. This check 
might help to catch an error in a future refactory.



##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/FilterSelectivityEstimator.java:
##########
@@ -937,12 +937,16 @@ private static Range<Float> 
convertRangeToClosedOpen(Range<Float> boundaries) {
    * @return the selectivity of "val1 &lt;= column &lt; val2"
    */
   static double rangedSelectivity(KllFloatsSketch kll, float val1, float val2) 
{
-    if (val1 >= val2) {
+    if (val1 >= val2 || val2 <= kll.getMinItem() || val1 > kll.getMaxItem()) {

Review Comment:
   Yes:
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityLessThanWhenLowerThanMin
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityLessThanOrEqualWhenLowerThanMin
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityGreaterThanWhenHigherThanMax
   * 
TestFilterSelectivityEstimator.testComputeRangePredicateSelectivityGreaterThanOrEqualWhenHigherThanMax
   * TestFilterSelectivityEstimator.testRangePredicateOnTimestamp
   * TestFilterSelectivityEstimator.testRangePredicateOnTimestampWithCast
   * TestFilterSelectivityEstimator.testRangePredicateOnDate
   * TestFilterSelectivityEstimator.testRangePredicateOnDateWithCast



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