Subject: [PATCH 8/8] batch build

From: Tomas Vondra <[email-masked]>
Date: Fri, 22 Jan 2021 01:11:08 +0100

---
 src/backend/access/brin/brin_minmax_multi.c | 204 ++++++++++++++++++--
 src/backend/access/brin/brin_tuple.c        |   1 +
 src/include/access/brin_tuple.h             |   2 +-
 3 files changed, 186 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 094ecd2be9..72cca6f26b 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -57,6 +57,7 @@
 #include "access/brin.h"
 #include "access/brin_internal.h"
 #include "access/brin_tuple.h"
+#include "access/hash.h"	/* XXX strange that it fails because of BRIN_AM_OID without this */
 #include "access/reloptions.h"
 #include "access/stratnum.h"
 #include "access/htup_details.h"
@@ -150,12 +151,23 @@ typedef struct MinMaxOptions
 typedef struct Ranges
 {
 	Oid		typid;
+	Oid		colloid;
+	AttrNumber	attno;
 
 	/* (2*nranges + nvalues) <= maxvalues */
 	int		nranges;	/* number of ranges in the array (stored) */
 	int		nvalues;	/* number of values in the data array (all) */
 	int		maxvalues;	/* maximum number of values (reloption) */
 
+	/*
+	 * Batch mode means we're simply appending values into new range,
+	 * without any expensive steps (sorting, deduplication, ...). The
+	 * buffer is sized to be much larger, but we keep the actual target
+	 * size so that when serializing values we can do the right thing.
+	 */
+	bool	batch_mode;
+	int		target_maxvalues;
+
 	/* values stored for this range - either raw values, or ranges */
 	Datum	values[FLEXIBLE_ARRAY_MEMBER];
 } Ranges;
@@ -375,6 +387,7 @@ range_deserialize(SerializedRanges *serialized)
 	range->nvalues = serialized->nvalues;
 	range->maxvalues = serialized->maxvalues;
 	range->typid = serialized->typid;
+	range->batch_mode = false;
 
 	typbyval = get_typbyval(serialized->typid);
 	typlen = get_typlen(serialized->typid);
@@ -844,11 +857,14 @@ fill_combine_ranges(CombineRange *cranges, int ncranges, Ranges *ranges)
 /*
  * Sort combine ranges using qsort (with BTLessStrategyNumber function).
  */
-static void
+static int
 sort_combine_ranges(FmgrInfo *cmp, Oid colloid,
-					CombineRange *cranges, int ncranges)
+					CombineRange *cranges, int ncranges,
+					bool deduplicate)
 {
-	compare_context cxt;
+	int				n;
+	int				i;
+	compare_context	cxt;
 
 	/* sort the values */
 	cxt.colloid = colloid;
@@ -856,6 +872,24 @@ sort_combine_ranges(FmgrInfo *cmp, Oid colloid,
 
 	qsort_arg(cranges, ncranges, sizeof(CombineRange),
 			  compare_combine_ranges, (void *) &cxt);
+
+	if (!deduplicate)
+		return ncranges;
+
+	/* optionally deduplicate the ranges */
+	n = 1;
+	for (i = 1; i < ncranges; i++)
+	{
+		if (compare_combine_ranges(&cranges[i-1], &cranges[i], (void *) &cxt))
+		{
+			if (i != n)
+				memcpy(&cranges[n], &cranges[i], sizeof(CombineRange));
+
+			n++;
+		}
+	}
+
+	return n;
 }
 
 /*
@@ -1026,26 +1060,40 @@ build_distances(FmgrInfo *distanceFn, Oid colloid,
  */
 static CombineRange *
 build_combine_ranges(FmgrInfo *cmp, Oid colloid, Ranges *ranges,
-					 Datum newvalue, int *nranges)
+					 bool addvalue, Datum newvalue, int *nranges,
+					 bool deduplicate)
 {
 	int				ncranges;
 	CombineRange   *cranges;
 
 	/* now do the actual merge sort */
-	ncranges = ranges->nranges + ranges->nvalues + 1;
+	ncranges = ranges->nranges + ranges->nvalues;
+
+	/* should we add an extra value? */
+	if (addvalue)
+		ncranges += 1;
+
 	cranges = (CombineRange *) palloc0(ncranges * sizeof(CombineRange));
-	*nranges = ncranges;
 
 	/* put the new value at the beginning */
-	cranges[0].minval = newvalue;
-	cranges[0].maxval = newvalue;
-	cranges[0].collapsed = true;
+	if (addvalue)
+	{
+		cranges[0].minval = newvalue;
+		cranges[0].maxval = newvalue;
+		cranges[0].collapsed = true;
 
-	/* then the regular and collapsed ranges */
-	fill_combine_ranges(&cranges[1], ncranges-1, ranges);
+		/* then the regular and collapsed ranges */
+		fill_combine_ranges(&cranges[1], ncranges-1, ranges);
+	}
+	else
+		fill_combine_ranges(cranges, ncranges, ranges);
 
 	/* and sort the ranges */
-	sort_combine_ranges(cmp, colloid, cranges, ncranges);
+	ncranges = sort_combine_ranges(cmp, colloid, cranges, ncranges,
+								   deduplicate);
+
+	/* remember how many cranges we built */
+	*nranges = ncranges;
 
 	return cranges;
 }
@@ -1074,6 +1122,7 @@ count_values(CombineRange *cranges, int ncranges)
 }
 #endif
 
+
 /*
  * reduce_combine_ranges
  *		reduce the ranges until the number of values is low enough
@@ -1250,6 +1299,25 @@ range_add_value(BrinDesc *bdesc, Oid colloid,
 
 	Assert(2*ranges->nranges + ranges->nvalues <= ranges->maxvalues);
 
+	Assert((ranges->nranges >= 0) && (ranges->nvalues >= 0) && (ranges->maxvalues >= 0));
+
+	/*
+	 * When batch-building, there should be no ranges. So either the
+	 * number of ranges is 0 or we're not in batching mode.
+	 */
+	Assert(!ranges->batch_mode || (ranges->nranges == 0));
+
+	/* When batch-building, just add it and we're done. */
+	if (ranges->batch_mode)
+	{
+		/* there has to be free space, if we've sized the struct */
+		Assert(ranges->nvalues < ranges->maxvalues);
+
+		ranges->values[ranges->nvalues++] = newval;
+
+		return true;
+	}
+
 	/*
 	 * Bail out if the value already is covered by the range.
 	 *
@@ -1322,7 +1390,9 @@ range_add_value(BrinDesc *bdesc, Oid colloid,
 					 ranges->nvalues);
 
 	/* OK build the combine ranges */
-	cranges = build_combine_ranges(cmpFn, colloid, ranges, newval, &ncranges);
+	cranges = build_combine_ranges(cmpFn, colloid, ranges,
+								   true, newval, &ncranges,
+								   false);
 
 	/* Reduce the ranges if needed */
 	if (ncranges > ranges->maxvalues)
@@ -1382,6 +1452,81 @@ range_add_value(BrinDesc *bdesc, Oid colloid,
 	return true;
 }
 
+/*
+ * Generate range representation of data collected during "batch mode".
+ * This is similar to reduce_combine_ranges, except that we can't assume
+ * the values are sorted and there may be duplicate values.
+ */
+static void
+compactify_ranges(BrinDesc *bdesc, Ranges *ranges, int max_values)
+{
+	FmgrInfo   *cmpFn,
+			   *distanceFn;
+
+	/* combine ranges */
+	CombineRange   *cranges;
+	int				ncranges;
+	DistanceValue  *distances;
+
+	MemoryContext	ctx;
+	MemoryContext	oldctx;
+
+	/*
+	 * This should only be used in batch mode, and there should be no
+	 * ranges, just individual values.
+	 */
+	Assert((ranges->batch_mode) && (ranges->nranges == 0));
+
+	/* we'll certainly need the comparator, so just look it up now */
+	cmpFn = minmax_multi_get_strategy_procinfo(bdesc, ranges->attno, ranges->typid,
+											   BTLessStrategyNumber);
+
+	/* and we'll also need the 'distance' procedure */
+	distanceFn = minmax_multi_get_procinfo(bdesc, ranges->attno, PROCNUM_DISTANCE);
+
+	/*
+	 * The distanceFn calls (which may internally call e.g. numeric_le) may
+	 * allocate quite a bit of memory, and we must not leak it. Otherwise
+	 * we'd have problems e.g. when building indexes. So we create a local
+	 * memory context and make sure we free the memory before leaving this
+	 * function (not after every call).
+	 */
+	ctx = AllocSetContextCreate(CurrentMemoryContext,
+								"minmax-multi context",
+								ALLOCSET_DEFAULT_SIZES);
+
+	oldctx = MemoryContextSwitchTo(ctx);
+
+	/* OK build the combine ranges */
+	cranges = build_combine_ranges(cmpFn, ranges->colloid, ranges,
+								   false, (Datum) 0, &ncranges,
+								   true);	/* deduplicate */
+
+	/* build array of gap distances and sort them in ascending order */
+	distances = build_distances(distanceFn, ranges->colloid, cranges, ncranges);
+
+	/*
+	 * Combine ranges until we get below max_values. We don't use any scale
+	 * factor, because this is used at the very end of "batch mode" and we
+	 * don't expect more tuples to be inserted soon.
+	 */
+	ncranges = reduce_combine_ranges(cranges, ncranges, distances,
+									  max_values, cmpFn, ranges->colloid);
+
+	Assert(count_values(cranges, ncranges) <= max_values);
+
+	/* decompose the combine ranges into regular ranges and single values */
+	store_combine_ranges(ranges, cranges, ncranges);
+
+	MemoryContextSwitchTo(oldctx);
+	MemoryContextDelete(ctx);
+
+	/* Check the ordering invariants are not violated (for both parts). */
+	AssertArrayOrder(cmpFn, ranges->colloid, ranges->values, ranges->nranges*2);
+	AssertArrayOrder(cmpFn, ranges->colloid, &ranges->values[ranges->nranges*2],
+					 ranges->nvalues);
+}
+
 Datum
 brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS)
 {
@@ -1799,10 +1944,16 @@ brin_minmax_multi_distance_inet(PG_FUNCTION_ARGS)
 }
 
 static void
-brin_minmax_multi_serialize(Datum src, Datum *dst)
+brin_minmax_multi_serialize(BrinDesc *bdesc, Datum src, Datum *dst)
 {
 	Ranges *ranges = (Ranges *) DatumGetPointer(src);
-	SerializedRanges *s = range_serialize(ranges);
+	SerializedRanges *s;
+
+	/* In batch mode, we need to compress the accumulated values. */
+	if (ranges->batch_mode)
+		compactify_ranges(bdesc, ranges, ranges->target_maxvalues);
+
+	s = range_serialize(ranges);
 	dst[0] = PointerGetDatum(s);
 }
 
@@ -1845,15 +1996,28 @@ brin_minmax_multi_add_value(PG_FUNCTION_ARGS)
 	/*
 	 * If this is the first non-null value, we need to initialize the range
 	 * list. Otherwise just extract the existing range list from BrinValues.
+	 *
+	 * When starting with an empty range, we assume this is a batch mode,
+	 * i.e. we size the buffer for the maximum possible number of items in
+	 * the range (based on range size and max number of items on a page).
+	 *
+	 * XXX This may require quite a bit of memory, so maybe we should use
+	 * some value in between. OTOH most tables will have much wider rows,
+	 * so the number of rows per page is much lower.
 	 */
 	if (column->bv_allnulls)
 	{
 		MemoryContext oldctx;
 
-		oldctx = MemoryContextSwitchTo(column->bv_context);
+		BlockNumber		pagesPerRange = BrinGetPagesPerRange(bdesc->bd_index);
 
-		ranges = minmax_multi_init(brin_minmax_multi_get_values(bdesc, opts));
+		oldctx = MemoryContextSwitchTo(column->bv_context);
+		ranges = minmax_multi_init(MaxHeapTuplesPerPage * pagesPerRange);
+		ranges->attno = attno;
+		ranges->colloid = colloid;
 		ranges->typid = attr->atttypid;
+		ranges->batch_mode = true;
+		ranges->target_maxvalues = brin_minmax_multi_get_values(bdesc, opts);
 
 		MemoryContextSwitchTo(oldctx);
 
@@ -2125,7 +2289,7 @@ brin_minmax_multi_union(PG_FUNCTION_ARGS)
 	oldctx = MemoryContextSwitchTo(ctx);
 
 	/* allocate and fill */
-	cranges = (CombineRange *)palloc0(ncranges * sizeof(CombineRange));
+	cranges = (CombineRange *) palloc0(ncranges * sizeof(CombineRange));
 
 	/* fill the combine ranges with entries for the first range */
 	fill_combine_ranges(cranges, ranges_a->nranges + ranges_a->nvalues,
@@ -2139,8 +2303,8 @@ brin_minmax_multi_union(PG_FUNCTION_ARGS)
 	cmpFn = minmax_multi_get_strategy_procinfo(bdesc, attno, attr->atttypid,
 											 BTLessStrategyNumber);
 
-	/* sort the combine ranges */
-	sort_combine_ranges(cmpFn, colloid, cranges, ncranges);
+	/* sort the combine ranges (don't deduplicate) */
+	sort_combine_ranges(cmpFn, colloid, cranges, ncranges, false);
 
 	/*
 	 * We've merged two different lists of ranges, so some of them may be
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index df820db029..bf8635d788 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -163,6 +163,7 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
 		if (tuple->bt_columns[keyno].bv_serialize)
 		{
 			tuple->bt_columns[keyno].bv_serialize(
+				brdesc,
 				tuple->bt_columns[keyno].bv_mem_value,
 				tuple->bt_columns[keyno].bv_values);
 		}
diff --git a/src/include/access/brin_tuple.h b/src/include/access/brin_tuple.h
index 66a6c3c792..e08998b71a 100644
--- a/src/include/access/brin_tuple.h
+++ b/src/include/access/brin_tuple.h
@@ -14,7 +14,7 @@
 #include "access/brin_internal.h"
 #include "access/tupdesc.h"
 
-typedef void (*brin_serialize_callback_type) (Datum src, Datum * dst);
+typedef void (*brin_serialize_callback_type) (BrinDesc *bdesc, Datum src, Datum * dst);
 
 /*
  * A BRIN index stores one index tuple per page range.  Each index tuple
-- 
2.26.2

--------------FA974B75A0C3E9CA18CE7207
Content-Type: application/x-shellscript;
 name="brin.sh"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="brin.sh"

--------------FA974B75A0C3E9CA18CE7207--