zabetak commented on code in PR #6746:
URL: https://github.com/apache/hive/pull/6746#discussion_r4120222072
##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestFilterSelectivityEstimator.java:
##########
@@ -1202,4 +1208,185 @@ private static long timestampMillis(String timestamp) {
private static long timestamp(String timestamp) {
return timestampMillis(timestamp) / 1000;
}
+
+ private static final int INTEGER_FIELD_INDEX = 6; // f_integer
+ private static final int DATE_FIELD_INDEX = 9; // f_date
+
+ private void setupMinMaxNoHistogram(float min, float max) {
+ setupMinMaxNoHistogram(min, max, 0);
+ }
+
+ private void setupMinMaxNoHistogram(float min, float max, long numNulls) {
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ stats.setRange(min, max);
+ stats.setNumNulls(numNulls);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ }
+
+ private RelNode createScanWithPlanner(HiveConf conf) {
+ RelOptPlanner planner = CalcitePlanner.createPlanner(conf);
+ RelOptCluster cluster = RelOptCluster.create(planner, REX_BUILDER);
+ RelBuilder relBuilder = HiveRelFactories.HIVE_BUILDER.create(cluster,
schemaMock);
+ HiveTableScan tableScan =
+ new HiveTableScan(cluster, cluster.traitSetOf(HiveRelNode.CONVENTION),
tableMock, "table", null, false, false);
+ return relBuilder.push(tableScan).build();
+ }
+
+ @Test
+ public void testComparisonMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
Review Comment:
Consider using the existing method for keeping tests uniform:
```
useFieldWithValues("f_integer", new float[] {0, 100}, null);
```
##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestFilterSelectivityEstimator.java:
##########
@@ -1202,4 +1208,185 @@ private static long timestampMillis(String timestamp) {
private static long timestamp(String timestamp) {
return timestampMillis(timestamp) / 1000;
}
+
+ private static final int INTEGER_FIELD_INDEX = 6; // f_integer
+ private static final int DATE_FIELD_INDEX = 9; // f_date
+
+ private void setupMinMaxNoHistogram(float min, float max) {
+ setupMinMaxNoHistogram(min, max, 0);
+ }
+
+ private void setupMinMaxNoHistogram(float min, float max, long numNulls) {
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ stats.setRange(min, max);
+ stats.setNumNulls(numNulls);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ }
+
+ private RelNode createScanWithPlanner(HiveConf conf) {
+ RelOptPlanner planner = CalcitePlanner.createPlanner(conf);
+ RelOptCluster cluster = RelOptCluster.create(planner, REX_BUILDER);
+ RelBuilder relBuilder = HiveRelFactories.HIVE_BUILDER.create(cluster,
schemaMock);
+ HiveTableScan tableScan =
+ new HiveTableScan(cluster, cluster.traitSetOf(HiveRelNode.CONVENTION),
tableMock, "table", null, false, false);
+ return relBuilder.push(tableScan).build();
+ }
+
+ @Test
+ public void testComparisonMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int50 = REX_BUILDER.makeLiteral(50,
TYPE_FACTORY.createSqlType(INTEGER), true);
Review Comment:
If you want to be sure that you get a literal and not a CAST over a literal
you must use `REX_BUILDER.makeLiteral(50, TYPE_FACTORY.createSqlType(INTEGER))`
method.
##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestFilterSelectivityEstimator.java:
##########
@@ -1202,4 +1208,185 @@ private static long timestampMillis(String timestamp) {
private static long timestamp(String timestamp) {
return timestampMillis(timestamp) / 1000;
}
+
+ private static final int INTEGER_FIELD_INDEX = 6; // f_integer
+ private static final int DATE_FIELD_INDEX = 9; // f_date
+
+ private void setupMinMaxNoHistogram(float min, float max) {
+ setupMinMaxNoHistogram(min, max, 0);
+ }
+
+ private void setupMinMaxNoHistogram(float min, float max, long numNulls) {
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ stats.setRange(min, max);
+ stats.setNumNulls(numNulls);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ }
Review Comment:
I think these methods are duplicating a bit what's already done in
`useFieldWithValues`. You can drop these and slightly modify the existing
method to make tests easier to read.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/FilterSelectivityEstimator.java:
##########
@@ -484,19 +484,164 @@ private Double
computeRangePredicateSelectivity(Supplier<Double> defaultSelectiv
}
final List<ColStatistics> colStats =
scan.getColStat(Collections.singletonList(inputRefIndex));
- if (colStats.isEmpty() || !isHistogramAvailable(colStats.get(0))) {
+ if (colStats.isEmpty()) {
return defaultSelectivity.get();
}
- final KllFloatsSketch kll =
KllFloatsSketch.heapify(Memory.wrap(colStats.get(0).getHistogram()));
- double rawSelectivity = rangedSelectivity(kll, boundaries);
+ final ColStatistics cs = colStats.get(0);
+ if (isHistogramAvailable(cs)) {
+ final KllFloatsSketch kll =
KllFloatsSketch.heapify(Memory.wrap(cs.getHistogram()));
+ double rawSelectivity = rangedSelectivity(kll, boundaries);
+ if (inverseBool) {
+ // when inverseBool == true, this is a NOT_BETWEEN and selectivity
must be inverted
+ // if there's a cast, the inversion is with respect to its codomain
(range of the values of the cast)
+ double typeRangeSelectivity = rangedSelectivity(kll, typeRange);
+ rawSelectivity = typeRangeSelectivity - rawSelectivity;
+ }
+ return scaleSelectivityToNullableValues(kll, rawSelectivity, scan);
+ }
+
+ if (isUniformWithinRangeEnabled() && hasUsableMinMax(cs)) {
+ RelDataType columnType =
scan.getRowType().getFieldList().get(inputRefIndex).getType();
+ Double uniformSelectivity = computeUniformRangeSelectivity(cs,
boundaries, scan, inverseBool, typeRange,
+ columnType);
+ if (uniformSelectivity != null) {
+ return uniformSelectivity;
+ }
+ }
+
+ return defaultSelectivity.get();
+ }
+
+ private boolean isUniformWithinRangeEnabled() {
+ HiveConfPlannerContext ctx =
+
childRel.getCluster().getPlanner().getContext().unwrap(HiveConfPlannerContext.class);
+ return ctx == null || ctx.isUniformWithinRange();
+ }
+
+ private static boolean hasUsableMinMax(ColStatistics cs) {
+ ColStatistics.Range range = cs.getRange();
+ return range != null && range.minValue != null && range.maxValue != null;
+ }
+
+ /**
+ * Converts column MIN/MAX statistics into the same numeric space used by
{@link #extractLiteral}.
+ * DATE column stats from HMS are stored as days since epoch; literals use
epoch seconds.
+ */
+ private static Optional<Range<Float>>
convertColRangeToFloatRange(ColStatistics cs, RelDataType columnType) {
+ ColStatistics.Range range = cs.getRange();
+ if (range == null || range.minValue == null || range.maxValue == null) {
+ return Optional.empty();
+ }
+ final Number minValue = range.minValue;
+ final Number maxValue = range.maxValue;
+ switch (columnType.getSqlTypeName()) {
+ case DATE:
+ return Optional.of(Range.closed((float) (minValue.longValue() * 86400L),
+ (float) (maxValue.longValue() * 86400L)));
+ case TINYINT:
+ case SMALLINT:
+ case INTEGER:
+ case BIGINT:
+ case FLOAT:
+ case DOUBLE:
+ case DECIMAL:
+ case TIMESTAMP:
+ return Optional.of(Range.closed(minValue.floatValue(),
maxValue.floatValue()));
Review Comment:
It appears that for some datatypes the logic here will do some truncation
when converting to float. The code in `StatsRulesProcFactory` behaves
differently so to some extend we will have some inconsistency between
CBO/non-CBO path. This may be acceptable for the time being but I am wondering
if need extra tests to ensure that the behavior will not be completely
off/wrong when dealing with BIGINT, DOUBLE, DECIMAL.
##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestFilterSelectivityEstimator.java:
##########
@@ -1202,4 +1208,185 @@ private static long timestampMillis(String timestamp) {
private static long timestamp(String timestamp) {
return timestampMillis(timestamp) / 1000;
}
+
+ private static final int INTEGER_FIELD_INDEX = 6; // f_integer
+ private static final int DATE_FIELD_INDEX = 9; // f_date
+
+ private void setupMinMaxNoHistogram(float min, float max) {
+ setupMinMaxNoHistogram(min, max, 0);
+ }
+
+ private void setupMinMaxNoHistogram(float min, float max, long numNulls) {
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ stats.setRange(min, max);
+ stats.setNumNulls(numNulls);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ }
+
+ private RelNode createScanWithPlanner(HiveConf conf) {
+ RelOptPlanner planner = CalcitePlanner.createPlanner(conf);
+ RelOptCluster cluster = RelOptCluster.create(planner, REX_BUILDER);
+ RelBuilder relBuilder = HiveRelFactories.HIVE_BUILDER.create(cluster,
schemaMock);
+ HiveTableScan tableScan =
+ new HiveTableScan(cluster, cluster.traitSetOf(HiveRelNode.CONVENTION),
tableMock, "table", null, false, false);
+ return relBuilder.push(tableScan).build();
+ }
+
+ @Test
+ public void testComparisonMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int50 = REX_BUILDER.makeLiteral(50,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, currentInputRef,
int50);
Review Comment:
There are various shortcuts for this already defined in this class so
consider using those as well where applicable.
```suggestion
RexNode filter = le(currentInputRef, int50);
```
##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestFilterSelectivityEstimator.java:
##########
@@ -1202,4 +1208,185 @@ private static long timestampMillis(String timestamp) {
private static long timestamp(String timestamp) {
return timestampMillis(timestamp) / 1000;
}
+
+ private static final int INTEGER_FIELD_INDEX = 6; // f_integer
+ private static final int DATE_FIELD_INDEX = 9; // f_date
+
+ private void setupMinMaxNoHistogram(float min, float max) {
+ setupMinMaxNoHistogram(min, max, 0);
+ }
+
+ private void setupMinMaxNoHistogram(float min, float max, long numNulls) {
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ stats.setRange(min, max);
+ stats.setNumNulls(numNulls);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ }
+
+ private RelNode createScanWithPlanner(HiveConf conf) {
+ RelOptPlanner planner = CalcitePlanner.createPlanner(conf);
+ RelOptCluster cluster = RelOptCluster.create(planner, REX_BUILDER);
+ RelBuilder relBuilder = HiveRelFactories.HIVE_BUILDER.create(cluster,
schemaMock);
+ HiveTableScan tableScan =
+ new HiveTableScan(cluster, cluster.traitSetOf(HiveRelNode.CONVENTION),
tableMock, "table", null, false, false);
+ return relBuilder.push(tableScan).build();
+ }
+
+ @Test
+ public void testComparisonMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int50 = REX_BUILDER.makeLiteral(50,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, currentInputRef,
int50);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.5, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxNoHistogramNoRange() {
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN,
currentInputRef, int3);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.3333333333333333,
estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonDateMinMaxNoHistogram() {
+ long minDays = LocalDate.parse("2020-11-01").toEpochDay();
+ long maxDays = LocalDate.parse("2020-11-07").toEpochDay();
+ stats = new ColStatistics();
+ stats.setHistogram(null);
+ stats.setRange(minDays, maxDays);
+ currentInputRef = REX_BUILDER.makeInputRef(scan, DATE_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(DATE_FIELD_INDEX));
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, currentInputRef,
+ literalDate("2020-11-04"));
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ // Epoch-second DATE bounds are stored as float; range intersection may
differ slightly from 0.5
+ Assert.assertEquals(0.5, estimator.estimateSelectivity(filter), 1e-3f);
+ }
+
+ @Test
+ public void testHistogramPreferredOverMinMax() {
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(0));
+ stats.setRange(0, 100);
+ RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN,
inputRef0, int3);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.6153846153846154,
estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testBetweenMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int20 = REX_BUILDER.makeLiteral(20,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode int40 = REX_BUILDER.makeLiteral(40,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter = REX_BUILDER.makeCall(HiveBetween.INSTANCE, boolFalse,
currentInputRef, int20, int40);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.2, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testNotBetweenMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int20 = REX_BUILDER.makeLiteral(20,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode int40 = REX_BUILDER.makeLiteral(40,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter = REX_BUILDER.makeCall(HiveBetween.INSTANCE, boolTrue,
currentInputRef, int20, int40);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.8, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxBelowMin() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN,
currentInputRef, int0);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxAboveMax() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int150 = REX_BUILDER.makeLiteral(150,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.GREATER_THAN,
currentInputRef, int150);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxLessThanOrEqualMax() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int100 = REX_BUILDER.makeLiteral(100,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, currentInputRef,
int100);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(1, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxGreaterThanOrEqualMin() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL,
currentInputRef, int0);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(1, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxWhenMinEqualsMaxLessThanOrEqual() {
+ setupMinMaxNoHistogram(5, 5);
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, currentInputRef,
int5);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(1, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxWhenMinEqualsMaxLessThan() {
+ setupMinMaxNoHistogram(5, 5);
+ RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN,
currentInputRef, int5);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxWithNulls() {
+ setupMinMaxNoHistogram(0, 100, 2);
+ doReturn((double) 20).when(tableMock).getRowCount();
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, currentInputRef,
int5);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.045, estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testComparisonMinMaxUniformFlagDisabled() {
+ setupMinMaxNoHistogram(0, 100);
+ HiveConf conf = new HiveConf();
+
conf.setBoolVar(HiveConf.ConfVars.HIVE_STATS_RANGE_SELECTIVITY_UNIFORM_DISTRIBUTION,
false);
+ RelNode localScan = createScanWithPlanner(conf);
+ RexNode localInputRef = REX_BUILDER.makeInputRef(localScan,
INTEGER_FIELD_INDEX);
+ doReturn(Collections.singletonList(stats)).when(tableMock)
+ .getColStat(Collections.singletonList(INTEGER_FIELD_INDEX));
+ RexNode filter =
REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, localInputRef,
int5);
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(localScan, mq);
+ Assert.assertEquals(0.3333333333333333,
estimator.estimateSelectivity(filter), DELTA);
+ }
+
+ @Test
+ public void testSearchTwoSidedMinMaxNoHistogram() {
+ setupMinMaxNoHistogram(0, 100);
+ RexNode int20 = REX_BUILDER.makeLiteral(20,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode int40 = REX_BUILDER.makeLiteral(40,
TYPE_FACTORY.createSqlType(INTEGER), true);
+ RexNode filter = REX_BUILDER.makeCall(SqlStdOperatorTable.AND,
+ REX_BUILDER.makeCall(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL,
currentInputRef, int20),
+ REX_BUILDER.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL,
currentInputRef, int40));
+ filter = simplify(filter);
+ Assert.assertEquals(SqlKind.SEARCH, filter.getKind());
+ FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
+ Assert.assertEquals(0.2, estimator.estimateSelectivity(filter), DELTA);
+ }
Review Comment:
It seems that there are no tests around edge conditions (notably
MIN/MAX_VALUE). I added the test below and it returns a selectivity of 0.0
which implies that there is a bug somewhere in the code.
```
@Test
public void testComparisonWithColRangeOnMaxValue() {
useFieldWithValues("f_float", new float[] {0, Float.MAX_VALUE}, null);
RexNode filter = le(currentInputRef, literalFloat(Float.MAX_VALUE / 2));
FilterSelectivityEstimator estimator = new
FilterSelectivityEstimator(scan, mq);
Assert.assertEquals(0.5, estimator.estimateSelectivity(filter), DELTA);
}
```
It seems that when we estimate column statistics
(org.apache.hadoop.hive.ql.stats.StatsUtils#estimateColStats) we are setting
the range to the min and max based on the colun type.
--
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]
