fix: count only live rows in the planner estimate

CHANGELOG.md

@@ -35,6 +35,23 @@ true until the next version shipped.
 
 ### Fixed
 
+- The planner estimate counts live rows, and reads the delete count in one
+  catalog scan rather than one per row group.
+
+  **`row_group.row_count` is physical occupancy.** It still counts rows that a
+  later `DELETE` marked in the delete vector, and the planner uses this callback
+  instead of `pg_class.reltuples`, so every scan of a heavily deleted table was
+  priced as if the deletes had not happened.
+
+  The count now comes from one indexed scan summing
+  `delete_vector.deleted_count` over the storage. The earlier shape walked each
+  group's bitmap a bit at a time, once per row group, on every plan of a
+  columnar relation.
+
+  Summing is exact rather than an approximation. `delete_vector` carries a
+  unique index on `(storage_id, group_number)`, so there is one row per group
+  and no two summands can count the same row. The comment that justified the
+  per-group fold said the opposite, and the schema forbids what it described.
 - Arrow import reads the temporal unit and carrier width the file declares,
   rather than assuming the ones our own exporter writes (#864, #865).
 

src/columnar.h

@@ -552,6 +552,8 @@ extern Snapshot PgColumnarCatalogSnapshot(Snapshot base);
 extern List *PgColumnarReadDeleteVectorList(uint64 storageId, uint64 stripeId,
 									 Snapshot snapshot);
 extern void PgColumnarUpsertDeleteVector(uint64 storageId, DeleteVectorMetadata *rm);
+extern uint64 PgColumnarGroupDeletedCount(uint64 storageId, NativeRowGroupMetadata *rg,
+									  Snapshot snapshot);
 
 /* -------------------------------------------------------------------------
  * writer (pgcolumnar_write_state.c)

src/columnar_delete_vector.c

@@ -497,3 +497,60 @@ PgColumnarDeleteVectorPromoteSubXact(SubTransactionId subid, SubTransactionId pa
 			buf->subid = parent;
 	}
 }
+
+/*
+ * PgColumnarGroupDeletedCount
+ *		How many of this row group's rows are deleted, under the given catalog
+ *		snapshot. Bits past the group's row count are ignored.
+ *
+ *		This walks the group's mask the same way the reader does when it builds
+ *		one (spec 7.5), which is why it lives beside that code rather than being
+ *		reimplemented at each caller.
+ *
+ *		It does NOT do so to avoid double-counting. An earlier version of this
+ *		comment said a group can have several delete_vector rows whose bitmaps
+ *		overlap, so summing deletedCount would count a row deleted twice. The
+ *		catalog forbids it: delete_vector carries a unique index on
+ *		(storage_id, group_number), so there is at most one row per group and
+ *		nothing to OR together. The planner estimate now sums deleted_count over
+ *		the storage in one scan for exactly that reason.
+ */
+uint64
+PgColumnarGroupDeletedCount(uint64 storageId, NativeRowGroupMetadata *rg,
+						  Snapshot snapshot)
+{
+	uint32		want = (uint32) ((rg->rowCount + 7) / 8);
+	char	   *mask;
+	List	   *rml;
+	ListCell   *mc;
+	uint64		deleted = 0;
+	uint32		b;
+
+	rml = PgColumnarReadDeleteVectorList(storageId, rg->groupNumber, snapshot);
+	if (rml == NIL)
+		return 0;
+
+	mask = palloc0(want > 0 ? want : 1);
+	foreach(mc, rml)
+	{
+		DeleteVectorMetadata *rm = (DeleteVectorMetadata *) lfirst(mc);
+
+		if (rm->bitmap == NULL || rm->bitmapLen == 0)
+			continue;
+		for (b = 0; b < rm->bitmapLen && b < want; b++)
+			mask[b] |= rm->bitmap[b];
+	}
+
+	for (b = 0; b < want; b++)
+	{
+		uint64		base = (uint64) b * 8;
+		int			i;
+
+		for (i = 0; i < 8; i++)
+			if (base + i < rg->rowCount && ((mask[b] >> i) & 1))
+				deleted++;
+	}
+
+	pfree(mask);
+	return deleted;
+}

src/columnar_metadata.c

@@ -1423,6 +1423,55 @@ PgColumnarStorageHasDeleteVector(uint64 storageId, Snapshot snapshot)
 	return found;
 }
 
+/*
+ * PgColumnarStorageDeletedCount
+ *		Total rows marked deleted across the whole storage, read from
+ *		delete_vector.deleted_count.
+ *
+ *		One index scan over the storage's delete_vector rows, summing a stored
+ *		integer, rather than one scan and a bitmap walk per row group. The
+ *		planner calls this on every plan of a columnar relation, so the cost is
+ *		paid per plan and not per query.
+ *
+ *		deleted_count is the number of set bits in that row's bitmap, maintained
+ *		where the bitmap is written, so summing it is exact rather than an
+ *		approximation -- the unique index on (storage_id, group_number) means
+ *		one row per group, so no two summands can count the same row.
+ */
+uint64
+PgColumnarStorageDeletedCount(uint64 storageId, Snapshot snapshot)
+{
+	Relation	rel = open_columnar_table("delete_vector", AccessShareLock);
+	ScanKeyData key[1];
+	SysScanDesc scan;
+	HeapTuple	tup;
+	TupleDesc	tupdesc = RelationGetDescr(rel);
+	uint64		total = 0;
+	Oid			dvIdx = pgcolumnar_index_oid("delete_vector_pkey");
+
+	ScanKeyInit(&key[0], Anum_delete_vector_storage_id, BTEqualStrategyNumber,
+				F_INT8EQ, Int64GetDatum((int64) storageId));
+	scan = systable_beginscan(rel, dvIdx, OidIsValid(dvIdx), snapshot, 1, key);
+	while (HeapTupleIsValid(tup = systable_getnext(scan)))
+	{
+		bool		isnull;
+		Datum		d = heap_getattr(tup, Anum_delete_vector_deleted_count,
+									 tupdesc, &isnull);
+
+		if (!isnull)
+		{
+			int32		n = DatumGetInt32(d);
+
+			if (n > 0)
+				total += (uint64) n;
+		}
+	}
+	systable_endscan(scan);
+	table_close(rel, AccessShareLock);
+
+	return total;
+}
+
 /*
  * delete_vector_chunk_lock_key
  *		Mix the identity of a chunk group into a 64-bit advisory-lock key. The

src/columnar_metadata.h

@@ -95,5 +95,6 @@ extern void PgColumnarDeleteProjectionDeclaration(Oid relid, const char *name);
 extern void PgColumnarDeleteProjectionDeclarationsForRel(Oid relid);
 
 extern bool PgColumnarStorageHasDeleteVector(uint64 storageId, Snapshot snapshot);
+extern uint64 PgColumnarStorageDeletedCount(uint64 storageId, Snapshot snapshot);
 
 #endif							/* PGCOLUMNAR_METADATA_H */

src/columnar_tableam.c

@@ -841,10 +841,53 @@ pgcolumnar_relation_estimate_size(Relation rel, int32 *attr_widths,
 	 * planner from mis-costing scans (spec 6, 9).
 	 */
 	snapshot = ActiveSnapshotSet() ? GetActiveSnapshot() : GetTransactionSnapshot();
-	rowGroupList = PgColumnarReadRowGroupList(storageId, PgColumnarCatalogSnapshot(snapshot));
+	snapshot = PgColumnarCatalogSnapshot(snapshot);
+	rowGroupList = PgColumnarReadRowGroupList(storageId, snapshot);
 
-	foreach(lc, rowGroupList)
-		liveRows += (double) ((NativeRowGroupMetadata *) lfirst(lc))->rowCount;
+	/*
+	 * row_group.row_count is the physical occupancy, including rows later
+	 * marked in delete_vector. The planner uses this callback instead of
+	 * pg_class.reltuples, so leaving those rows in *tuples prices every scan
+	 * as if DELETE had not happened.
+	 */
+	{
+		uint64		physicalRows = 0;
+		uint64		deleted;
+
+		foreach(lc, rowGroupList)
+		{
+			NativeRowGroupMetadata *rg = (NativeRowGroupMetadata *) lfirst(lc);
+
+			physicalRows += rg->rowCount;
+		}
+
+		/*
+		 * One catalog scan summing delete_vector.deleted_count, not one scan
+		 * and a bitmap walk per row group. This runs on every plan of a
+		 * columnar relation, so a per-group fold is paid per plan.
+		 *
+		 * Summing is exact, not an approximation: the unique index on
+		 * (storage_id, group_number) means one delete_vector row per group, so
+		 * no two summands can count the same row. An earlier comment here
+		 * justified the per-group fold by saying a group can have several
+		 * delete_vector rows whose bitmaps overlap. The catalog forbids that,
+		 * and the expensive path was defended by a premise the schema rules
+		 * out.
+		 *
+		 * The clamp is against the storage total rather than per group, which
+		 * is what the per-group clamp was doing in aggregate. It matters only
+		 * if the count is ever inconsistent with the row groups, and an
+		 * estimate must not go negative.
+		 *
+		 * Both reads take the same catalog snapshot as the row-group list
+		 * above, deliberately, so the count cannot straddle two snapshots and
+		 * report more deletes than there are rows.
+		 */
+		deleted = PgColumnarStorageDeletedCount(storageId, snapshot);
+		if (deleted > physicalRows)
+			deleted = physicalRows;
+		liveRows = (double) (physicalRows - deleted);
+	}
 
 	*pages = Max(nblocks, 1);
 	*tuples = Max(liveRows, 0);

src/columnar_vector.c

@@ -2929,60 +2929,6 @@ pgcolumnar_agg_finalize(PgColumnarAggSpec *spec, bool *isnull)
 	return (Datum) 0;
 }
 
-/*
- * pgcolumnar_group_deleted_count
- *		How many of this row group's rows are deleted, under the given catalog
- *		snapshot. A group can have several delete_vector rows, whose bitmaps
- *		overlap, so they are OR'd before counting rather than summed -- summing
- *		deletedCount across entries would double-count a row deleted twice (spec
- *		7.5, and the same combining the reader does when it builds a group's mask).
- *		Bits past the group's row count are ignored.
- */
-static uint64
-pgcolumnar_group_deleted_count(uint64 storageId, NativeRowGroupMetadata *rg,
-							 Snapshot snap)
-{
-	uint32		want = (uint32) ((rg->rowCount + 7) / 8);
-	char	   *mask;
-	List	   *rml;
-	ListCell   *mc;
-	uint64		deleted = 0;
-	uint32		b;
-
-	rml = PgColumnarReadDeleteVectorList(storageId, rg->groupNumber, snap);
-	if (rml == NIL)
-		return 0;
-
-	mask = palloc0(want > 0 ? want : 1);
-	foreach(mc, rml)
-	{
-		DeleteVectorMetadata *rm = (DeleteVectorMetadata *) lfirst(mc);
-
-		if (rm->bitmap == NULL || rm->bitmapLen == 0)
-			continue;
-		for (b = 0; b < rm->bitmapLen && b < want; b++)
-			mask[b] |= rm->bitmap[b];
-	}
-
-	/*
-	 * Count set bits only up to rowCount. The last byte of the bitmap can carry
-	 * bits beyond the group's final row, and counting those would report more
-	 * rows deleted than the group holds.
-	 */
-	for (b = 0; b < want; b++)
-	{
-		uint64		base = (uint64) b * 8;
-		int			i;
-
-		for (i = 0; i < 8; i++)
-			if (base + i < rg->rowCount && ((mask[b] >> i) & 1))
-				deleted++;
-	}
-
-	pfree(mask);
-	return deleted;
-}
-
 /*
  * pgcolumnar_fill_native_metadata_agg
  *		Answer an ungrouped, unfiltered aggregate over a native (PGCN v1) table
@@ -3065,7 +3011,7 @@ pgcolumnar_fill_native_metadata_agg(PgColumnarAggScanState *state, int *ndirty)
 		int			a;
 
 		if (anyDeletes)
-			deleted = pgcolumnar_group_deleted_count(storageId, rg, snap);
+			deleted = PgColumnarGroupDeletedCount(storageId, rg, snap);
 
 		if (deleted > 0)
 		{