Fix chunk skipping range check bug

Chunk skipping based on column stats had a convoluted way
of detecting if query clauses were hitting a chunk in certain
cases. This change makes things more straight forward and
adds missing test cases for range checking.

Author: antekresic

src/hypertable_restrict_info.c

@@ -175,81 +175,6 @@ dimension_restrict_info_open_add(DimensionRestrictInfoOpen *dri, StrategyNumber
 	return restriction_added;
 }
 
-/*
- * for DIMENSION_TYPE_STATS entries, we need to use regular bounded strategies.
- * We can have multiple entries satisfying the inputs. It basically becomes a
- * problem of:
- *
- * "given an input value, find all ranges which encompass that value"
- *
- * For "column >= constant" (e.g id >= 9), the check has to be:
- *
- * start_range >= 9 OR end_range >= 9 (second check covers +INF)
- *
- * For "column <= constant" (e.g id <= 90), the check has to be:
- *
- * start_range <= 90 OR end_range <= 90 (first check covers -INF)
- *
- * For "column == constant" (e.g id = 9)
- *
- * start_range <= 9 OR end_range >= 9 (covers +INF to -INF)
- */
-static bool
-dimension_restrict_info_range_add(DimensionRestrictInfoOpen *dri, StrategyNumber strategy,
-								  Oid collation, DimensionValues *dimvalues)
-{
-	ListCell *item;
-	bool restriction_added = false;
-
-	/* can't handle IN/ANY with multiple values */
-	if (dimvalues->use_or && list_length(dimvalues->values) > 1)
-		return false;
-
-	foreach (item, dimvalues->values)
-	{
-		Oid restype;
-		Datum datum = ts_dimension_transform_value(dri->base.dimension,
-												   collation,
-												   PointerGetDatum(lfirst(item)),
-												   dimvalues->type,
-												   &restype);
-		int64 value = ts_time_value_to_internal_or_infinite(datum, restype);
-
-		switch (strategy)
-		{
-			case BTLessEqualStrategyNumber:
-			case BTLessStrategyNumber: /* e.g: id <= 90 */
-				if (dri->upper_strategy == InvalidStrategy || value < dri->upper_bound)
-				{
-					dri->upper_strategy = strategy;
-					dri->upper_bound = value;
-					restriction_added = true;
-				}
-				break;
-			case BTGreaterEqualStrategyNumber:
-			case BTGreaterStrategyNumber: /* e.g: id >= 9 */
-				if (dri->lower_strategy == InvalidStrategy || value > dri->lower_bound)
-				{
-					dri->lower_strategy = strategy;
-					dri->lower_bound = value;
-					restriction_added = true;
-				}
-				break;
-			case BTEqualStrategyNumber: /* e.g: id = 9 */
-				dri->lower_bound = value;
-				dri->upper_bound = value;
-				dri->lower_strategy = BTEqualStrategyNumber;
-				dri->upper_strategy = BTEqualStrategyNumber;
-				restriction_added = true;
-				break;
-			default:
-				/* unsupported strategy */
-				break;
-		}
-	}
-	return restriction_added;
-}
-
 static List *
 dimension_restrict_info_get_partitions(DimensionRestrictInfoClosed *dri, Oid collation,
 									   List *values)
@@ -330,10 +255,10 @@ dimension_restrict_info_add(DimensionRestrictInfo *dri, int strategy, Oid collat
 													values);
 		case DIMENSION_TYPE_STATS:
 			/* we reuse the DimensionRestrictInfoOpen structure for these */
-			return dimension_restrict_info_range_add((DimensionRestrictInfoOpen *) dri,
-													 strategy,
-													 collation,
-													 values);
+			return dimension_restrict_info_open_add((DimensionRestrictInfoOpen *) dri,
+													strategy,
+													collation,
+													values);
 		case DIMENSION_TYPE_CLOSED:
 			return dimension_restrict_info_closed_add((DimensionRestrictInfoClosed *) dri,
 													  strategy,

src/ts_catalog/chunk_column_stats.c

@@ -1071,7 +1071,6 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
 	ScanIterator it;
 	List *chunkids = NIL;
 	DimensionRestrictInfoOpen *open;
-	bool check_both = false;
 
 	Assert(dri && dri->dimension->type == DIMENSION_TYPE_STATS);
 
@@ -1082,25 +1081,6 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
 
 	open = (DimensionRestrictInfoOpen *) dri;
 
-	/*
-	 * Since we need to find chunks with overlapping ranges. We use checks on both the
-	 * lower_bound and the upper_bound.
-	 */
-	if (open->upper_strategy != InvalidStrategy && open->lower_strategy != InvalidStrategy)
-		check_both = true;
-
-	if (open->upper_strategy == InvalidStrategy)
-	{
-		open->upper_strategy = open->lower_strategy;
-		open->upper_bound = open->lower_bound;
-	}
-
-	if (open->lower_strategy == InvalidStrategy)
-	{
-		open->lower_strategy = open->upper_strategy;
-		open->lower_bound = open->upper_bound;
-	}
-
 	/*
 	 * We need to get all chunks matching the hypertable ID and the column name.
 	 */
@@ -1143,18 +1123,6 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
 			goto done;
 		}
 
-		/*
-		 * If both upper and lower bounds have been specified then we need to check
-		 * if that range overlaps (subset, superset, intersect) with the current fd entry
-		 * values
-		 */
-		if (check_both)
-		{
-			/* range is before or after our fd range_start/range_end values */
-			if (open->upper_bound < fd->range_start || open->lower_bound > fd->range_end)
-				goto done;
-		}
-
 		/*
 		 * All data is in int8 format so we do regular comparisons. Also, it's an OR
 		 * check so prepare to short circuit if one evaluates to true.
@@ -1166,78 +1134,37 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
 		{
 			case BTLessEqualStrategyNumber: /* e.g: id <= 90 */
 			{
-				/* range_end is exclusive, so check accordingly */
-				if ((fd->range_end - 1) <= open->upper_bound)
-					matched = check_both ? false : true;
+				matched = fd->range_start <= open->upper_bound;
 			}
 			break;
 			case BTLessStrategyNumber: /* e.g: id < 90 */
 			{
-				/* range_end is exclusive, so check accordingly */
-				if ((fd->range_end - 1) < open->upper_bound)
-					matched = check_both ? false : true;
-			}
-			break;
-			case BTGreaterEqualStrategyNumber: /* e.g: id >= 90 */
-			{
-				/* range_end is exclusive, so check accordingly */
-				if ((fd->range_end - 1) >= open->upper_bound)
-					matched = check_both ? false : true;
-			}
-			break;
-			case BTGreaterStrategyNumber: /* e.g: id > 9 */
-			{
-				/* range_end is exclusive, so check accordingly */
-				if ((fd->range_end - 1) > open->upper_bound)
-					matched = check_both ? false : true;
-			}
-			break;
-			case BTEqualStrategyNumber: /* e.g: id == 9 */
-			{
-				/* need to check for both range_start and range_end */
-				if (fd->range_start <= open->lower_bound &&
-					(fd->range_end - 1) >= open->upper_bound)
-					matched = true;
+				matched = fd->range_start < open->upper_bound;
 			}
 			break;
 			default:
-				/* unsupported strategy */
+				open->upper_strategy = InvalidStrategy;
 				break;
 		}
 
-		if (matched)
+		if (open->upper_strategy != InvalidStrategy && !matched)
 			goto done;
 
 		/* range_end checks didn't match, check for range_start now */
 		switch (open->lower_strategy)
 		{
-			case BTLessEqualStrategyNumber:
-			{
-				if (fd->range_start <= open->lower_bound)
-					matched = true;
-			}
-			break;
-			case BTLessStrategyNumber:
-			{
-				if (fd->range_start < open->lower_bound)
-					matched = true;
-			}
-			break;
 			case BTGreaterEqualStrategyNumber:
 			{
-				if (fd->range_start >= open->lower_bound)
-					matched = true;
+				/* range_end is exclusive */
+				matched = (fd->range_end - 1) >= open->lower_bound;
 			}
 			break;
 			case BTGreaterStrategyNumber:
 			{
-				/* range_start is inclusive */
-				if (fd->range_start >= open->lower_bound)
-					matched = true;
+				/* range_end is exclusive */
+				matched = (fd->range_end - 1) > open->lower_bound;
 			}
 			break;
-			case BTEqualStrategyNumber:
-				/* already handled above with upper_strategy */
 			default:
 				/* unsupported strategy */
 				break;