Cost a columnar index scan's per-row heap fetch (#355)

src/columnar.h

@@ -182,6 +182,14 @@ extern bool columnar_enable_read_stream;	/* stream/prefetch block reads (PG17+)
 extern bool columnar_enable_index_only_scan;	/* allow index-only scans (gap 28) */
 extern bool columnar_bulk_parallel_writer;	/* internal: parallel_copy loader skips the storage-row creation lock (#300) */
 extern bool columnar_enable_projection_scan;	/* scan a covering projection (gap 26) */
+extern bool columnar_enable_index_fetch_penalty;	/* price a columnar index scan's per-row fetch (#355) */
+
+/*
+ * Statement-scoped by-row-number fetch cache cap (columnar_reader.c). Named here
+ * so the index-fetch cost model (#355) can tell when a stripe is too wide to be
+ * retained across fetches and must be treated as re-decoded per row.
+ */
+#define COLUMNAR_FETCH_CACHE_MAX_BYTES	(32 * 1024 * 1024)
 
 /* issue #5: concurrent unique-key insert serialization */
 extern bool columnar_enable_unique_lock;	/* serialize same-key inserters */

src/columnar_customscan.c

@@ -26,11 +26,14 @@
  */
 #include "columnar.h"
 
+#include <math.h>
+
 #include "access/parallel.h"
 #include "access/relation.h"
 #include "access/relscan.h"
 #include "access/stratnum.h"
 #include "access/table.h"
+#include "catalog/pg_statistic.h"
 #include "catalog/pg_type.h"
 #include "storage/shm_toc.h"
 #include "commands/explain.h"
@@ -53,12 +56,15 @@
 #include "optimizer/restrictinfo.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
+#include "utils/syscache.h"
 #include "utils/typcache.h"
 
 /* GUC: use the columnar custom scan path (spec 8.3) */
 bool		columnar_enable_custom_scan = true;
 /* GUC: let the planner scan a covering projection instead of the base (gap 26) */
 bool		columnar_enable_projection_scan = true;
+/* GUC: price a columnar index scan's per-row heap fetch (#355) */
+bool		columnar_enable_index_fetch_penalty = true;
 
 static set_rel_pathlist_hook_type prev_set_rel_pathlist_hook = NULL;
 
@@ -487,6 +493,157 @@ columnar_choose_projection(PlannerInfo *root, RelOptInfo *rel, Oid relid)
 	return best;
 }
 
+/*
+ * columnar_index_correlation
+ *		|correlation| of the index's leading key against heap (row-number) order,
+ *		read from pg_statistic exactly as btcostestimate does (selfuncs.c). A value
+ *		near 1 means rows an ordered index scan visits are already clustered into a
+ *		few row groups; near 0 means they are scattered across all of them.
+ *
+ * We take only the leading key and, for a multi-column index, damp it: trailing
+ * keys reorder within a leading-key run, so the run's rows land less tightly than
+ * the leading correlation alone suggests. Returns 0.0 (treat as scattered, the
+ * conservative-for-us direction) whenever a statistic is missing -- an expression
+ * key, no ANALYZE, no correlation slot.
+ */
+static double
+columnar_index_correlation(IndexOptInfo *index, Oid heapRelid)
+{
+	AttrNumber	attno;
+	Oid			sortop;
+	HeapTuple	st;
+	AttStatsSlot sslot;
+	double		corr = 0.0;
+
+	if (index->indexkeys == NULL || index->nkeycolumns < 1)
+		return 0.0;
+	attno = index->indexkeys[0];
+	if (attno <= 0)				/* 0 == expression key, no column statistic */
+		return 0.0;
+
+	sortop = get_opfamily_member(index->opfamily[0], index->opcintype[0],
+								 index->opcintype[0], BTLessStrategyNumber);
+	if (!OidIsValid(sortop))
+		return 0.0;
+
+	st = SearchSysCache3(STATRELATTINH, ObjectIdGetDatum(heapRelid),
+						 Int16GetDatum(attno), BoolGetDatum(false));
+	if (!HeapTupleIsValid(st))
+		return 0.0;
+
+	if (get_attstatsslot(&sslot, st, STATISTIC_KIND_CORRELATION, sortop,
+						 ATTSTATSSLOT_NUMBERS))
+	{
+		if (sslot.nnumbers == 1)
+		{
+			corr = sslot.numbers[0];
+			if (index->reverse_sort[0])
+				corr = -corr;
+			if (index->nkeycolumns > 1)
+				corr *= 0.75;
+		}
+		free_attstatsslot(&sslot);
+	}
+	ReleaseSysCache(st);
+	return corr;
+}
+
+/*
+ * columnar_scan_nproj
+ *		number of distinct base-relation columns a scan of this rel has to
+ *		materialize: the ones in the output target plus the ones in the
+ *		restriction clauses. Used to size the per-group decode cost, since the
+ *		fetch decodes a column at a time. Never returns less than 1.
+ */
+static int
+columnar_scan_nproj(RelOptInfo *rel, Index rti)
+{
+	Bitmapset  *attrs = NULL;
+	ListCell   *lc;
+	int			n;
+
+	pull_varattnos((Node *) rel->reltarget->exprs, rti, &attrs);
+	foreach(lc, rel->baserestrictinfo)
+		pull_varattnos((Node *) lfirst_node(RestrictInfo, lc)->clause, rti, &attrs);
+	n = bms_num_members(attrs);
+	bms_free(attrs);
+	return (n > 0) ? n : 1;
+}
+
+/*
+ * columnar_index_fetch_penalty
+ *		extra cost to add to a heap-fetching columnar index scan for the row-group
+ *		decodes its per-row fetches force. Core's cost_index prices a heap fetch as
+ *		a page or two; a columnar fetch decodes the whole row group the row lives in
+ *		(columnar_reader.c, ColumnarReadRowByNumber), which is why the planner picks
+ *		an index scan for an unclustered ORDER BY and then runs for minutes (#355).
+ *
+ * A statement-scoped cache (issue #143) means a group is decoded once per scan, not
+ * once per row, so the count that matters is how many *distinct* groups the fetched
+ * rows fall into:
+ *
+ *   - fully clustered (rho -> 1, or a TID-ordered bitmap heap scan): the rows sit in
+ *     ceil(rows / R) adjacent groups, the floor.
+ *   - fully scattered (rho -> 0): every fetched row can land in its own group, up to
+ *     one decode per row, the ceiling.
+ *   - between: interpolate on rho^2, the share of ordering variance the correlation
+ *     explains, matching how cost_index already blends min and max page cost.
+ *
+ * The one exception is the fetch cache cliff (#359): when one group's decoded width
+ * exceeds the cache cap it is never retained, so every fetch re-decodes regardless of
+ * clustering -- the ceiling, unconditionally. (When #359 makes that overflow
+ * proportional rather than total, this branch should soften with it.)
+ *
+ * decode_per_group is one group's cost: its pages read once, plus the per-value
+ * decode of R rows across the columns the scan needs.
+ */
+static Cost
+columnar_index_fetch_penalty(RelOptInfo *rel, double rows, double rho,
+							 int nproj, bool tid_ordered)
+{
+	double		R = (double) columnar_stripe_row_limit;
+	double		N = (rel->tuples > 0) ? rel->tuples : rows;
+	double		n_groups,
+				pages_per_stripe,
+				decode_per_group;
+	double		groups_min,
+				groups_max,
+				groups_decoded,
+				csq;
+
+	if (rows <= 0 || R < 1)
+		return 0.0;
+
+	n_groups = ceil(N / R);
+	if (n_groups < 1)
+		n_groups = 1;
+	pages_per_stripe = ceil((double) rel->pages / n_groups);
+	if (pages_per_stripe < 1)
+		pages_per_stripe = 1;
+	decode_per_group = seq_page_cost * pages_per_stripe
+		+ cpu_operator_cost * R * (double) nproj;
+
+	groups_min = ceil(rows / R);
+	groups_max = rows;
+	if (tid_ordered)
+		groups_decoded = groups_min;
+	else
+	{
+		csq = rho * rho;
+		if (csq > 1.0)
+			csq = 1.0;
+		groups_decoded = groups_min + (groups_max - groups_min) * (1.0 - csq);
+	}
+
+	/* #359 cliff: a group too wide to cache is re-decoded on every fetch */
+	if ((double) rel->reltarget->width * R > (double) COLUMNAR_FETCH_CACHE_MAX_BYTES)
+		groups_decoded = groups_max;
+
+	if (groups_decoded < 0)
+		groups_decoded = 0;
+	return groups_decoded * decode_per_group;
+}
+
 /*
  * ColumnarSetRelPathlist
  *		set_rel_pathlist_hook: for a columnar base relation, replace the
@@ -687,6 +844,55 @@ ColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti,
 			add_partial_path(rel, &ppath->path);
 		}
 	}
+
+	/*
+	 * Price the per-row heap fetch of the surviving index and bitmap paths
+	 * (#355). This runs last, after every add_path above, on purpose: it mutates
+	 * total_cost in place, which unsorts rel->pathlist, and no add_path may see an
+	 * unsorted list. add_path's dominance test compares each pair directly and so
+	 * is order-independent; only its insertion position depends on the sort, and
+	 * set_cheapest -- which core runs right after this hook -- rescans the whole
+	 * list, so the final choice reflects the updated costs.
+	 *
+	 * Only non-parameterized paths are touched. A parameterized index scan is a
+	 * nested-loop inner side rescanned per outer row; the fetch cache spans those
+	 * rescans (it is released at executor end, not per rescan), so the distinct-
+	 * group count this model assumes for a single pass understates the reuse and
+	 * would over-penalize the join. #355 is the standalone ordering/lookup case,
+	 * which is where param_info is NULL.
+	 *
+	 * total_cost only, never startup_cost: the fetch cost is paid as rows are
+	 * pulled, so a LIMIT that stops the scan early pays proportionally, which the
+	 * planner models by fractioning (total - startup).
+	 */
+	if (columnar_enable_index_fetch_penalty)
+	{
+		int			nproj = columnar_scan_nproj(rel, rti);
+
+		foreach(lc, rel->pathlist)
+		{
+			Path	   *p = (Path *) lfirst(lc);
+
+			if (p->param_info != NULL)
+				continue;
+			if (p->pathtype == T_IndexScan)
+			{
+				IndexPath  *ip = castNode(IndexPath, p);
+				double		rho = columnar_index_correlation(ip->indexinfo,
+															 rte->relid);
+
+				p->total_cost += columnar_index_fetch_penalty(rel, p->rows, rho,
+															  nproj, false);
+			}
+			else if (p->pathtype == T_BitmapHeapScan)
+			{
+				/* a bitmap heap scan fetches in TID (row-number) order */
+				p->total_cost += columnar_index_fetch_penalty(rel, p->rows, 1.0,
+															  nproj, true);
+			}
+			/* T_IndexOnlyScan and the custom scans do no heap fetch */
+		}
+	}
 }
 
 /* -------------------------------------------------------------------------

src/columnar_reader.c

@@ -1791,8 +1791,9 @@ ColumnarFreeLivenessCache(ColumnarLivenessCache *cache)
  * rather than derived from the stored byte length: the stored form is encoded and
  * compressed, so a group of wide text decodes to many times its size on disk, and
  * sizing from disk would overshoot worst on exactly the tables this helps most.
+ * COLUMNAR_FETCH_CACHE_MAX_BYTES is defined in columnar.h so the index-fetch cost
+ * model (#355) can name the same cap.
  */
-#define COLUMNAR_FETCH_CACHE_MAX_BYTES	(32 * 1024 * 1024)
 
 typedef struct ColumnarFetchGroup
 {

src/columnar_tableam.c

@@ -2363,6 +2363,19 @@ _PG_init(void)
 							 0,
 							 NULL, NULL, NULL);
 
+	DefineCustomBoolVariable("pgcolumnar.enable_index_fetch_penalty",
+							 "Add the cost of the row-group decodes a columnar index "
+							 "scan's per-row heap fetches force (#355).",
+							 "A columnar heap fetch decodes the whole row group the row "
+							 "lives in, so an unclustered ordered index scan can cost far "
+							 "more than core's per-page estimate. Off restores the "
+							 "unpenalized planner behaviour.",
+							 &columnar_enable_index_fetch_penalty,
+							 true,
+							 PGC_USERSET,
+							 0,
+							 NULL, NULL, NULL);
+
 	DefineCustomBoolVariable("pgcolumnar.bulk_parallel_writer",
 							 "Internal. Set by pgcolumnar.parallel_copy loader workers "
 							 "so they skip the storage-row creation lock when the row "

test/analyze_stats.sh

@@ -328,4 +328,73 @@ check "ANALYZE on a wide table is not many times a full scan of it" \
 		'BEGIN { print (s > 0 && a < s * 20) ? "yes" : "no (" a "ms against a " s "ms scan)" }')" \
 	"yes"
 
+# --- 6. the fetch cost keeps the planner off an unclustered ordered index (#355) --
+#
+# An ordered index scan on a columnar table pays for the order by fetching each row
+# by number, and each fetch decodes the whole row group the row lives in. When the
+# ordering column is unclustered those rows are scattered across every group, so the
+# scan decodes the table many times over -- but core prices the fetch as a page or
+# two and picks the index to avoid a sort. columnar_index_fetch_penalty adds the
+# decode cost, so a sort over the scan wins instead. Measured on the bench, an
+# unclustered ORDER BY that took minutes on the index dropped to seconds once it
+# sorted.
+#
+# The checks are behavioural (plan shape), and paired: the same query is planned
+# with the penalty on and off, so the second is the premise of the first -- if the
+# planner would not have taken the index without the penalty there is nothing for it
+# to have prevented. random_page_cost is set to the SSD value the penalty has to
+# overcome; parallelism is off so the plan shape is deterministic.
+O355_ROWS=${PGC_O355_ROWS:-300000}
+# scat is a full permutation of 0..N-1 (48271 is coprime to N), so it is maximally
+# unclustered against row order -- correlation ~0. The multiply is done in bigint;
+# g*48271 overflows int4 well before g reaches N.
+psql_run "DROP TABLE IF EXISTS o355;
+	CREATE TABLE o355 (id int, scat int, pad text) USING pgcolumnar;
+	INSERT INTO o355 SELECT g, ((g::bigint * 48271) % $O355_ROWS)::int, repeat('p', 48)
+		FROM generate_series(1, $O355_ROWS) g;
+	CREATE INDEX o355_scat ON o355 (scat);
+	CREATE INDEX o355_id ON o355 (id);
+	ANALYZE o355;" >/dev/null
+
+# SET and EXPLAIN must share one q call (one psql session) for the GUC to apply to
+# the plan; q does not pass -q, so strip the SET command tags it echoes.
+ord_setup="SET max_parallel_workers_per_gather = 0; SET random_page_cost = 1.0;"
+plan_of() { q "$1" | grep -v '^SET$'; }
+
+# premise: without the penalty the planner takes the scattered index for ordering
+plan_off="$(plan_of "${ord_setup} SET pgcolumnar.enable_index_fetch_penalty = off;
+	EXPLAIN (COSTS off) SELECT * FROM o355 ORDER BY scat;")"
+echo "-- ORDER BY scat, penalty off: $(printf '%s' "$plan_off" | grep -m1 -E 'Scan|Sort')"
+check "without the fetch penalty an unclustered ORDER BY takes the index (#355 premise)" \
+	"$(grep -q 'Index Scan using o355_scat' <<<"$plan_off" && echo yes \
+		|| echo "no ($(printf '%s' "$plan_off" | head -1))")" \
+	"yes"
+
+# with the penalty (on by default) the same query sorts instead of fetching per row
+plan_on="$(plan_of "${ord_setup} EXPLAIN (COSTS off) SELECT * FROM o355 ORDER BY scat;")"
+echo "-- ORDER BY scat, penalty on: $(printf '%s' "$plan_on" | grep -m1 -E 'Scan|Sort')"
+check "the fetch penalty makes an unclustered ORDER BY sort rather than fetch per row (#355)" \
+	"$(  grep -qE 'Sort' <<<"$plan_on" && ! grep -q 'Index Scan using o355_scat' <<<"$plan_on" \
+		&& echo yes || echo "no ($(printf '%s' "$plan_on" | head -1))")" \
+	"yes"
+
+# safety: a clustered ordering column is still cheap to fetch, so the penalty must
+# not cost it out of the index -- this is the direction that would silently regress.
+plan_cl="$(plan_of "${ord_setup} EXPLAIN (COSTS off) SELECT * FROM o355 ORDER BY id;")"
+echo "-- ORDER BY id (clustered), penalty on: $(printf '%s' "$plan_cl" | grep -m1 -E 'Scan|Sort')"
+check "the fetch penalty leaves a clustered ORDER BY on its index (#355 must not over-fire)" \
+	"$(grep -q 'Index Scan using o355_id' <<<"$plan_cl" && echo yes \
+		|| echo "no ($(printf '%s' "$plan_cl" | head -1))")" \
+	"yes"
+
+# safety: a selective point lookup must still take the index (guards #171 under the
+# penalty, on a table that also carries a scattered secondary index)
+o355_target=$(( O355_ROWS / 2 ))
+plan_pt="$(plan_of "${ord_setup} EXPLAIN (COSTS off) SELECT * FROM o355 WHERE id = $o355_target;")"
+echo "-- point lookup on id, penalty on: $(printf '%s' "$plan_pt" | grep -m1 -E 'Scan|Sort')"
+check "the fetch penalty leaves a selective point lookup on the index (#355 vs #171)" \
+	"$(grep -qE 'Index (Only )?Scan|Bitmap Heap Scan' <<<"$plan_pt" && echo yes \
+		|| echo "no ($(printf '%s' "$plan_pt" | head -1))")" \
+	"yes"
+
 pgc_summary