Let a fetch by row number ask for only the columns it needs (part of #157)

src/columnar.h

@@ -528,6 +528,26 @@ extern void ColumnarFreeLivenessCache(ColumnarLivenessCache *cache);
 extern bool ColumnarReadRowByNumber(Relation rel, Snapshot snapshot,
 									uint64 rowNumber, Datum *values, bool *nulls);
 
+/*
+ * Decode exactly the columns in `needed` (0-based attnums); every other column
+ * reads as null. An empty or NULL set decodes nothing, which is what it says:
+ * this deliberately has no "NULL means all" convention, because a Bitmapset
+ * cannot distinguish empty from NULL, so a caller whose computed set came out
+ * empty would silently get the opposite of what it asked for. For every column
+ * call ColumnarReadRowByNumber, which takes no set.
+ *
+ * Decoding every column regardless makes a wide table exceed the fetch cache's
+ * size cap, so the entry is dropped after every fetch and the group is decoded
+ * again for the next row (issue #157).
+ */
+extern bool ColumnarReadRowByNumberCols(Relation rel, Snapshot snapshot,
+										uint64 rowNumber, Datum *values,
+										bool *nulls, Bitmapset *needed);
+
+/* Is the row visible? Decodes nothing. */
+extern bool ColumnarRowIsLive(Relation rel, Snapshot snapshot,
+							  uint64 rowNumber);
+
 /* -------------------------------------------------------------------------
  * Decoded chunk group (columnar_vector.c aggregate path)
  *

src/columnar_projection.c

@@ -342,14 +342,11 @@ columnar_read_projection(PG_FUNCTION_ARGS)
 	MemoryContext oldContext;
 	FmgrInfo   *outFns;
 	int			ncols;
-	int			tnatts;
 	int			i;
 	Snapshot	snap;
 	ColumnarReadState *readState;
 	Datum	   *rvals;
 	bool	   *rnulls;
-	Datum	   *basevals;
-	bool	   *basenulls;
 	uint64		projRowNum;
 
 	if (PG_ARGISNULL(0) || PG_ARGISNULL(1))
@@ -393,7 +390,6 @@ columnar_read_projection(PG_FUNCTION_ARGS)
 						projname, get_rel_name(relid))));
 
 	ncols = proj->columnsLen;
-	tnatts = RelationGetDescr(rel)->natts;
 
 	/* projection storage layout: [rownumber int8, projcol1..projcolK] */
 	projTupdesc = CreateTemplateTupleDesc(ncols + 1);
@@ -427,9 +423,6 @@ columnar_read_projection(PG_FUNCTION_ARGS)
 	snap = GetActiveSnapshot();
 	rvals = palloc(sizeof(Datum) * (ncols + 1));
 	rnulls = palloc(sizeof(bool) * (ncols + 1));
-	basevals = palloc(sizeof(Datum) * tnatts);
-	basenulls = palloc(sizeof(bool) * tnatts);
-
 	readState = ColumnarBeginReadWithStorage(rel, snap, proj->projStorageId,
 											 projTupdesc, NULL, NULL, 0, NULL);
 
@@ -440,8 +433,13 @@ columnar_read_projection(PG_FUNCTION_ARGS)
 		Datum		result;
 		bool		resnull = false;
 
-		/* deletes/visibility come from the base */
-		if (!ColumnarReadRowByNumber(rel, snap, baseRow, basevals, basenulls))
+		/*
+		 * Only the base row's visibility is wanted here -- every value this loop
+		 * emits comes from the projection's own storage. Reconstructing the whole
+		 * base row to answer that decoded every column and threw all of it away
+		 * (issue #157).
+		 */
+		if (!ColumnarRowIsLive(rel, snap, baseRow))
 			continue;
 
 		initStringInfo(&buf);
@@ -505,6 +503,7 @@ columnar_reconstruct_via_projection(PG_FUNCTION_ARGS)
 	bool	   *rnulls;
 	Datum	   *basevals;
 	bool	   *basenulls;
+	Bitmapset  *uncovered = NULL;
 	uint64		projRowNum;
 
 	if (PG_ARGISNULL(0) || PG_ARGISNULL(1))
@@ -601,6 +600,18 @@ columnar_reconstruct_via_projection(PG_FUNCTION_ARGS)
 	basevals = palloc(sizeof(Datum) * tnatts);
 	basenulls = palloc(sizeof(bool) * tnatts);
 
+	/*
+	 * The base row is only read for the columns the projection does not carry,
+	 * so those are the only ones worth decoding (issue #157).
+	 */
+	for (i = 0; i < tnatts; i++)
+	{
+		if (TupleDescAttr(tableDesc, i)->attisdropped)
+			continue;
+		if (covered[i] < 0)
+			uncovered = bms_add_member(uncovered, i);
+	}
+
 	readState = ColumnarBeginReadWithStorage(rel, snap, proj->projStorageId,
 											 projTupdesc, NULL, NULL, 0, NULL);
 
@@ -612,8 +623,9 @@ columnar_reconstruct_via_projection(PG_FUNCTION_ARGS)
 		bool		resnull = false;
 		bool		first = true;
 
-		/* fetch the base row (liveness + any non-covered columns) */
-		if (!ColumnarReadRowByNumber(rel, snap, baseRow, basevals, basenulls))
+		/* fetch the base row: liveness, and only the columns not covered */
+		if (!ColumnarReadRowByNumberCols(rel, snap, baseRow, basevals,
+										 basenulls, uncovered))
 			continue;
 
 		initStringInfo(&buf);

src/columnar_reader.c

@@ -1633,9 +1633,27 @@ columnar_fetch_group_slot(uint64 storageId, uint64 groupNumber, bool *hit)
 	return victim;
 }
 
-bool
-ColumnarReadRowByNumber(Relation rel, Snapshot snapshot, uint64 rowNumber,
-						Datum *values, bool *nulls)
+/*
+ * columnar_fetch_row
+ *		Shared worker behind the three fetch entry points below.
+ *
+ *		Which columns to decode is said two ways, and deliberately not one.
+ *		allColumns is an explicit flag; needed is a set of 0-based attribute
+ *		numbers consulted only when it is false.
+ *
+ *		The obvious single-argument form -- a set where NULL means "all" -- cannot
+ *		be made safe, because a Bitmapset does not distinguish empty from NULL: an
+ *		empty one *is* NULL. A caller that computes its set and finds nothing in it
+ *		would then silently ask for every column, which is the exact opposite, and
+ *		no assertion can catch it because the two cases are the same value.
+ *		A column outside it is not read, not decoded and not indexed, and comes
+ *		back null. wantValues == false stops as soon as liveness is settled,
+ *		without touching the group's bytes at all.
+ */
+static bool
+columnar_fetch_row(Relation rel, Snapshot snapshot, uint64 rowNumber,
+				   Datum *values, bool *nulls, bool allColumns,
+				   Bitmapset *needed, bool wantValues)
 {
 	uint64		storageId = ColumnarStorageId(rel);
 	TupleDesc	tupdesc = RelationGetDescr(rel);
@@ -1711,6 +1729,18 @@ ColumnarReadRowByNumber(Relation rel, Snapshot snapshot, uint64 rowNumber,
 		}
 	}
 
+	/*
+	 * Liveness is fully settled here: it depends only on the row group covering
+	 * the row and on the delete vector. A caller that asks nothing else is done,
+	 * without the group's bytes being read or a single column decoded (#157).
+	 */
+	if (!wantValues)
+	{
+		MemoryContextSwitchTo(oldContext);
+		MemoryContextDelete(tmp);
+		return true;
+	}
+
 	/*
 	 * The group's bytes and its decoded columns are the expensive part and the
 	 * part that repeats across fetches of the same group, so they come from the
@@ -1813,6 +1843,19 @@ ColumnarReadRowByNumber(Relation rel, Snapshot snapshot, uint64 rowNumber,
 		char	   *cursor;
 		uint64		present;
 
+		/*
+		 * A column the caller did not ask for is neither decoded nor indexed,
+		 * and reads as null rather than being left untouched: a caller that
+		 * projects and then reads outside its projection gets a null instead of
+		 * whatever the array happened to hold.
+		 */
+		if (!allColumns && !bms_is_member(c, needed))
+		{
+			values[c] = (Datum) 0;
+			nulls[c] = true;
+			continue;
+		}
+
 		if (cc == NULL)
 		{
 			values[c] = getmissingattr(tupdesc, c + 1, &nulls[c]);
@@ -1903,6 +1946,55 @@ ColumnarReadRowByNumber(Relation rel, Snapshot snapshot, uint64 rowNumber,
 	return true;
 }
 
+/*
+ * ColumnarReadRowByNumber
+ *		Reconstruct every column of the row addressed by a row number. False when
+ *		the row is not visible.
+ */
+bool
+ColumnarReadRowByNumber(Relation rel, Snapshot snapshot, uint64 rowNumber,
+						Datum *values, bool *nulls)
+{
+	return columnar_fetch_row(rel, snapshot, rowNumber, values, nulls,
+							  true, NULL, true);
+}
+
+/*
+ * ColumnarReadRowByNumberCols
+ *		Decode exactly the columns in `needed`; every other column reads as null.
+ *		An empty or NULL set therefore decodes nothing, which is what it says
+ *		rather than a silent "everything" -- for every column, call
+ *		ColumnarReadRowByNumber, which takes no set and cannot be misread.
+ *
+ *		Decoding every column whatever the caller wanted is not merely wasted
+ *		work on a wide table. The decoded bytes are measured against the fetch
+ *		cache's size cap, so the entry is dropped after every fetch and the group
+ *		is decoded again for the next row -- the behaviour the cache exists to
+ *		remove (issue #157).
+ */
+bool
+ColumnarReadRowByNumberCols(Relation rel, Snapshot snapshot, uint64 rowNumber,
+							Datum *values, bool *nulls, Bitmapset *needed)
+{
+	return columnar_fetch_row(rel, snapshot, rowNumber, values, nulls,
+							  false, needed, true);
+}
+
+/*
+ * ColumnarRowIsLive
+ *		Is the row visible? Decodes nothing.
+ *
+ *		columnar_index_delete_tuples asks exactly this, once per candidate index
+ *		tuple on a path nbtree drives during deletion, and answered it by
+ *		reconstructing every column and freeing the result unread.
+ */
+bool
+ColumnarRowIsLive(Relation rel, Snapshot snapshot, uint64 rowNumber)
+{
+	return columnar_fetch_row(rel, snapshot, rowNumber, NULL, NULL,
+							  false, NULL, false);
+}
+
 void
 ColumnarRescanRead(ColumnarReadState *readState)
 {

src/columnar_tableam.c

@@ -31,7 +31,8 @@
 #include "optimizer/plancat.h"
 #include "port/atomics.h"
 #include "storage/bufmgr.h"
-#if PG_VERSION_NUM >= 180000
+#if PG_VERSION_NUM >= 170000
+/* the read-stream ANALYZE rework landed in PG17, and so did this header */
 #include "storage/read_stream.h"
 #endif
 #include "storage/lmgr.h"
@@ -581,7 +582,14 @@ columnar_analyze_set_slice(ColumnarAnalyzeState *st, BlockNumber blockno)
 	}
 }
 
-#if PG_VERSION_NUM >= 180000
+/*
+ * The block comes from a read stream from PG17 and as a plain BlockNumber
+ * before that. columnar_compat.h supplies the parameter list and splits at the
+ * same major; these two must agree, and when they did not, PG17 took the
+ * pre-17 branch and failed to compile on a `blockno` its signature does not
+ * have.
+ */
+#if PG_VERSION_NUM >= 170000
 static bool
 columnar_scan_analyze_next_block(COLUMNAR_ANALYZE_NEXT_BLOCK_ARGS)
 {
@@ -848,23 +856,23 @@ columnar_index_delete_tuples(Relation rel, TM_IndexDeleteOp *delstate)
 {
 	Snapshot	snapshot = ActiveSnapshotSet() ? GetActiveSnapshot()
 		: GetTransactionSnapshot();
-	TupleDesc	tupdesc = RelationGetDescr(rel);
-	Datum	   *values = (Datum *) palloc(sizeof(Datum) * tupdesc->natts);
-	bool	   *nulls = (bool *) palloc(sizeof(bool) * tupdesc->natts);
 	int			i;
 
 	for (i = 0; i < delstate->ndeltids; i++)
 	{
 		uint64		rowNumber =
 			ColumnarItemPointerToRowNumber(&delstate->deltids[i].tid);
-		bool		live = ColumnarReadRowByNumber(rel, snapshot, rowNumber,
-												   values, nulls);
 
-		delstate->status[delstate->deltids[i].id].knowndeletable = !live;
+		/*
+		 * Only liveness matters here, and ColumnarRowIsLive decodes nothing to
+		 * answer it. This used to reconstruct every column of the row and then
+		 * free the result unread, once per candidate index tuple, on a path
+		 * nbtree drives during deletion (issue #157).
+		 */
+		delstate->status[delstate->deltids[i].id].knowndeletable =
+			!ColumnarRowIsLive(rel, snapshot, rowNumber);
 	}
 
-	pfree(values);
-	pfree(nulls);
 	return InvalidTransactionId;
 }
 #endif