fix: a rewrite's own projection ids are not foreign reservations (#345)

src/columnar.h

@@ -517,6 +517,7 @@ typedef struct ColumnarWriteState ColumnarWriteState;
 extern ColumnarWriteState *ColumnarGetWriteState(Relation rel);
 extern int ColumnarWriteStateStripeCount(ColumnarWriteState *ws);
 extern uint64 *ColumnarWriteStateStripeIds(ColumnarWriteState *ws, int *n);
+extern uint64 *ColumnarWriteStateProjStripeIds(ColumnarWriteState *ws, int *n);
 extern uint64 ColumnarWriteRow(ColumnarWriteState *writeState, Relation rel,
 							   Datum *values, bool *nulls);
 extern void ColumnarProjectionFanoutRow(Relation rel, ColumnarWriteState *baseWs,

src/columnar_vacuum.c

@@ -336,11 +336,45 @@ record_online_sorted_extent(Relation rel, uint64 storageId,
 	if (nOurs <= 0)
 		return;					/* this rewrite reserved nothing */
 
-	/* our own reservations, ascending */
+	/*
+	 * Our own reservations, ascending -- including the ones our projection
+	 * fan-out drew (#345). A projection writes through its own write state but
+	 * reserves from this relation's stripe counter, so its ids interleave with
+	 * ours and would otherwise read as foreign reservations, truncating the run
+	 * at the first projection flush. They are ours: the same transaction drew
+	 * them. Only ids at or above our own first one are taken, so anything drawn
+	 * before this rewrite began is still excluded.
+	 */
 	ours = (uint64 *) palloc(sizeof(uint64) * nOurs);
 	memcpy(ours, all + stripeMark, sizeof(uint64) * nOurs);
 	qsort(ours, nOurs, sizeof(uint64), uint64_cmp);
 
+	{
+		int			nProj = 0;
+		uint64	   *projIds = ColumnarWriteStateProjStripeIds(writeState, &nProj);
+
+		if (nProj > 0)
+		{
+			uint64		lo = ours[0];
+			uint64	   *merged = (uint64 *) palloc(sizeof(uint64) * (nOurs + nProj));
+			int			m = nOurs;
+			int			j;
+
+			memcpy(merged, ours, sizeof(uint64) * nOurs);
+			for (j = 0; j < nProj; j++)
+			{
+				if (projIds[j] >= lo)
+					merged[m++] = projIds[j];
+			}
+			pfree(ours);
+			ours = merged;
+			nOurs = m;
+			qsort(ours, nOurs, sizeof(uint64), uint64_cmp);
+		}
+		if (projIds != NULL)
+			pfree(projIds);
+	}
+
 	/* the consecutive run of our own ids, starting at the lowest */
 	runEnd = ours[0];
 	for (i = 1; i < nOurs; i++)

src/columnar_write_state.c

@@ -1457,6 +1457,63 @@ typedef struct ColumnarProjWriter
 	MemoryContext rowCtx;		/* reset after each stripe flush: row datums */
 	ColumnarWriteState *innerWs;	/* reused stripe encoder for this projection */
 } ColumnarProjWriter;
+/*
+ * ColumnarWriteStateProjStripeIds
+ *		The stripe ids this write state's projection fan-out drew (#345).
+ *
+ *		A projection writes through its own inner write state but reserves from
+ *		the BASE relation's stripe counter, because ColumnarWriteRow is called
+ *		with the base relation (see flush_proj_writer). Its groups are recorded
+ *		under the projection's own storage id, so they never appear in the base
+ *		relation's row group list.
+ *
+ *		That combination is why the caller needs these separately. To
+ *		record_online_sorted_extent, an id drawn by its own projection fan-out is
+ *		indistinguishable from one taken by another session: both leave a gap in
+ *		the base write state's ids. Treating the former as foreign truncated the
+ *		ordered run at the first projection flush, so a fully reclustered table
+ *		with a projection reported almost all of itself as decayed.
+ *
+ *		Returns a palloc'd array in the caller's context, or NULL when this write
+ *		state has no projection writers.
+ */
+uint64 *
+ColumnarWriteStateProjStripeIds(ColumnarWriteState *ws, int *n)
+{
+	ListCell   *lc;
+	uint64	   *ids = NULL;
+	int			total = 0;
+	int			k = 0;
+
+	*n = 0;
+	if (ws->projWriters == NIL)
+		return NULL;
+
+	foreach(lc, ws->projWriters)
+	{
+		ColumnarProjWriter *w = (ColumnarProjWriter *) lfirst(lc);
+
+		if (w->innerWs != NULL)
+			total += w->innerWs->nReservedStripeIds;
+	}
+	if (total == 0)
+		return NULL;
+
+	ids = (uint64 *) palloc(sizeof(uint64) * total);
+	foreach(lc, ws->projWriters)
+	{
+		ColumnarProjWriter *w = (ColumnarProjWriter *) lfirst(lc);
+		int			i;
+
+		if (w->innerWs == NULL)
+			continue;
+		for (i = 0; i < w->innerWs->nReservedStripeIds; i++)
+			ids[k++] = w->innerWs->reservedStripeIds[i];
+	}
+	*n = k;
+	return ids;
+}
+
 
 /*
  * columnar_build_write_state

test/recluster_extent.sh

@@ -144,6 +144,36 @@ check "the id-drawn-below counts still cover every stored row" \
 	"$(raw "SELECT (sorted_rows + appended_rows = (SELECT sum(rowcount) FROM pgcolumnar.stats('d')))::text
 			FROM pgcolumnar.sort_status('d');")" "true"
 
+# ------------------------------- a projection must not read as foreign (#345)
+
+# The rewrite's projection fan-out writes through its own write state but draws
+# stripe ids from THIS relation's counter, and records its groups under the
+# projection's own storage id. So its ids interleave with the rewrite's own and
+# are invisible in the base relation's group list -- exactly what a foreign
+# reservation looks like. Treating them as foreign truncated the ordered run at
+# the first projection flush, so a table that had just been fully reclustered
+# reported almost all of itself as decayed.
+#
+# Both arms are identical except for the projection, so the projection is
+# established as the cause rather than assumed.
+for arm in noproj withproj; do
+	raw "DROP TABLE IF EXISTS pr_$arm;" >/dev/null
+	raw "CREATE TABLE pr_$arm (id int, k int, v text) USING pgcolumnar;" >/dev/null
+	raw "SELECT pgcolumnar.set_options('pr_$arm', stripe_row_limit => 20000);" >/dev/null
+	raw "INSERT INTO pr_$arm SELECT g, ((g::bigint * 7919) % 100000)::int, 'v' || g
+	     FROM generate_series(1, 200000) g;" >/dev/null
+	if [ "$arm" = withproj ]; then
+		E="$(raw "SELECT pgcolumnar.add_projection('pr_$arm', 'p_$arm', ARRAY['k','id'], ARRAY['k']);")"
+		case "$E" in *ERROR*) echo "      add_projection failed: $E";; esac
+	fi
+	raw "SELECT pgcolumnar.recluster('pr_$arm', 'id', 'k');" >/dev/null
+	S="$(raw "SELECT sorted_rows FROM pgcolumnar.sort_status('pr_$arm');")"
+	A="$(raw "SELECT appended_rows FROM pgcolumnar.sort_status('pr_$arm');")"
+	echo "      $arm: sorted $S, appended $A"
+	check "$arm: a full recluster claims every row" "$S" "200000"
+	check "$arm: a full recluster leaves nothing appended" "$A" "0"
+done
+
 # ---------------------------------------------------- the uncontended case
 
 # With no concurrent writer the rewrite orders everything, so the run covers the