refactoring of function select_range_partitions()

funbringer · funbringer · commit 1204842fc919 · 2017-02-20T15:18:14.000+03:00
diff --git a/src/pg_pathman.c b/src/pg_pathman.c
@@ -455,41 +455,43 @@ select_range_partitions(const Datum value,
 						const int strategy,
 						WrapperNode *result)
 {
-	const RangeEntry   *current_re;
-	bool				lossy = false,
-						is_less,
-						is_greater;
+	bool	lossy = false,
+			is_less,
+			is_greater;
 
 #ifdef USE_ASSERT_CHECKING
-	bool				found = false;
-	int					counter = 0;
+	bool	found = false;
+	int		counter = 0;
 #endif
 
-	int					i,
-						startidx = 0,
-						endidx = nranges - 1,
-						cmp_min,
-						cmp_max;
+	int		startidx = 0,
+			endidx = nranges - 1,
+			cmp_min,
+			cmp_max,
+			i;
+
+	Bound	value_bound = MakeBound(value); /* convert value to Bound */
+
 
 	/* Initial value (no missing partitions found) */
 	result->found_gap = false;
 
-	/* Check boundaries */
+	/* Check 'ranges' array */
 	if (nranges == 0)
 	{
 		result->rangeset = NIL;
 		return;
 	}
+
+	/* Check corner cases */
 	else
 	{
 		Assert(ranges);
 		Assert(cmp_func);
 
-		/* Corner cases */
-		cmp_min = IsInfinite(&ranges[startidx].min) ?
-			1 : DatumGetInt32(FunctionCall2(cmp_func, value, BoundGetValue(&ranges[startidx].min)));
-		cmp_max = IsInfinite(&ranges[endidx].max) ?
-			-1 : DatumGetInt32(FunctionCall2(cmp_func, value, BoundGetValue(&ranges[endidx].max)));
+		/* Compare 'value' to absolute MIN and MAX bounds */
+		cmp_min = cmp_bounds(cmp_func, &value_bound, &ranges[startidx].min);
+		cmp_max = cmp_bounds(cmp_func, &value_bound, &ranges[endidx].max);
 
 		if ((cmp_min <= 0 && strategy == BTLessStrategyNumber) ||
 			(cmp_min < 0 && (strategy == BTLessEqualStrategyNumber ||
@@ -529,21 +531,16 @@ select_range_partitions(const Datum value,
 	/* Binary search */
 	while (true)
 	{
+		Assert(ranges);
 		Assert(cmp_func);
 
+		/* Calculate new pivot */
 		i = startidx + (endidx - startidx) / 2;
 		Assert(i >= 0 && i < nranges);
 
-		current_re = &ranges[i];
-
-		cmp_min = IsInfinite(&current_re->min) ?
-						1 :
-						FunctionCall2(cmp_func, value,
-									  BoundGetValue(&current_re->min));
-		cmp_max = IsInfinite(&current_re->max) ?
-						-1 :
-						FunctionCall2(cmp_func, value,
-									  BoundGetValue(&current_re->max));
+		/* Compare 'value' to current MIN and MAX bounds */
+		cmp_min = cmp_bounds(cmp_func, &value_bound, &ranges[i].min);
+		cmp_max = cmp_bounds(cmp_func, &value_bound, &ranges[i].max);
 
 		is_less = (cmp_min < 0 || (cmp_min == 0 && strategy == BTLessStrategyNumber));
 		is_greater = (cmp_max > 0 || (cmp_max >= 0 && strategy != BTLessStrategyNumber));
@@ -556,13 +553,14 @@ select_range_partitions(const Datum value,
 				lossy = false;
 			else
 				lossy = true;
+
 #ifdef USE_ASSERT_CHECKING
 			found = true;
 #endif
 			break;
 		}
 
-		/* If we still haven't found partition then it doesn't exist */
+		/* Indices have met, looks like there's no partition */
 		if (startidx >= endidx)
 		{
 			result->rangeset = NIL;
@@ -579,6 +577,7 @@ select_range_partitions(const Datum value,
 		Assert(++counter < 100);
 	}
 
+	/* Should've been found by now */
 	Assert(found);
 
 	/* Filter partitions */
@@ -638,8 +637,8 @@ wrapper_make_expression(WrapperNode *wrap, int index, bool *alwaysTrue)
 
 	*alwaysTrue = false;
 	/*
-	 * TODO: use faster algorithm using knowledge that we enumerate indexes
-	 * sequntially.
+	 * TODO: use faster algorithm using knowledge
+	 * that we enumerate indexes sequntially.
 	 */
 	found = irange_list_find(wrap->rangeset, index, &lossy);
 
@@ -670,16 +669,19 @@ wrapper_make_expression(WrapperNode *wrap, int index, bool *alwaysTrue)
 
 				arg = wrapper_make_expression((WrapperNode *) lfirst(lc),
 											  index, &childAlwaysTrue);
+
 #ifdef USE_ASSERT_CHECKING
 				/*
-				 * We shouldn't get there for always true clause under OR and
-				 * always false clause under AND.
+				 * We shouldn't get there for always true clause
+				 * under OR and always false clause under AND.
 				 */
 				if (expr->boolop == OR_EXPR)
 					Assert(!childAlwaysTrue);
+
 				if (expr->boolop == AND_EXPR)
 					Assert(arg || childAlwaysTrue);
 #endif
+
 				if (arg)
 					args = lappend(args, arg);
 			}
diff --git a/src/relation_info.h b/src/relation_info.h
@@ -79,12 +79,15 @@ cmp_bounds(FmgrInfo *cmp_func, const Bound *b1, const Bound *b2)
 {
 	if (IsMinusInfinity(b1) || IsPlusInfinity(b2))
 		return -1;
+
 	if (IsMinusInfinity(b2) || IsPlusInfinity(b1))
 		return 1;
 
 	Assert(cmp_func);
 
-	return FunctionCall2(cmp_func, BoundGetValue(b1), BoundGetValue(b2));
+	return DatumGetInt32(FunctionCall2(cmp_func,
+									   BoundGetValue(b1),
+									   BoundGetValue(b2)));
 }
 
 

Original file line number	Diff line number	Diff line change
`@@ -79,12 +79,15 @@ cmp_bounds(FmgrInfo cmp_func, const Bound b1, const Bound *b2)`
`79`	`79`	`{`
`80`	`80`	`if (IsMinusInfinity(b1) \|\| IsPlusInfinity(b2))`
`81`	`81`	`return -1;`
	`82`	`+`
`82`	`83`	`if (IsMinusInfinity(b2) \|\| IsPlusInfinity(b1))`
`83`	`84`	`return 1;`
`84`	`85`
`85`	`86`	`Assert(cmp_func);`
`86`	`87`
`87`		`- return FunctionCall2(cmp_func, BoundGetValue(b1), BoundGetValue(b2));`
	`88`	`+ return DatumGetInt32(FunctionCall2(cmp_func,`
	`89`	`+ BoundGetValue(b1),`
	`90`	`+ BoundGetValue(b2)));`
`88`	`91`	`}`
`89`	`92`
`90`	`93`