test: megabyte-scale values are stored exactly (#1341)

docs/limitations.md

@@ -779,7 +779,9 @@ therefore still be cheaper as a scan.
 ## Indexes
 
 - Stale index entries left by deletes and updates are filtered on fetch and
-  reclaimed by `REINDEX`, not removed opportunistically.
+  reclaimed by `REINDEX`, not removed opportunistically. `REINDEX` keeps the entry
+  of a deleted row only while a transaction that began before the delete is still
+  running, as it does on heap.
 - `CREATE INDEX CONCURRENTLY` (the concurrent validate path) and partial
   block-range index builds are not supported.
 

src/columnar_reader.c

@@ -296,6 +296,15 @@ struct PgColumnarReadState
 	bool		emitDeleted;
 	bool		lastRowDeleted;
 
+	/*
+	 * With emitDeleted, a row deleted under THIS snapshot too is dead to every
+	 * transaction, and is not returned (#1407). The index build passes one that
+	 * sees only transactions older than every running snapshot.
+	 */
+	Snapshot	deadHorizon;
+	char	   *deadMask;
+	uint32		deadMaskLen;
+
 	MemoryContext readContext;		/* whole scan */
 	MemoryContext stripeContext;	/* reset per stripe */
 	MemoryContext groupContext;		/* reset per chunk group (decompressed) */
@@ -2833,6 +2842,17 @@ pgcolumnar_native_load_group(PgColumnarReadState *rs)
 										&rs->nativeDeleteMask,
 										&rs->nativeDeleteMaskLen);
 	}
+	rs->deadMask = NULL;
+	rs->deadMaskLen = 0;
+	if (rs->deadHorizon != NULL && rs->nativeDeleteMask != NULL)
+	{
+		List	   *deadList = PgColumnarReadDeleteVectorList(rs->storageId,
+													   rg->groupNumber,
+													   rs->deadHorizon);
+
+		pgcolumnar_merge_delete_vectors(deadList, (uint32) ((rg->rowCount + 7) / 8),
+										&rs->deadMask, &rs->deadMaskLen);
+	}
 
 	/*
 	 * The chunk metadata is read before the data (#338) because it carries the
@@ -3458,6 +3478,9 @@ pgcolumnar_native_next_row(PgColumnarReadState *rs, Datum *values, bool *nulls,
 
 		if (deleted && !rs->emitDeleted)
 			continue;			/* row deleted: cursors advanced, do not emit */
+		if (deleted && rs->deadMask != NULL &&
+			dv_row_deleted(rs->deadMask, rs->deadMaskLen, rs->rowInGroup - 1))
+			continue;			/* dead to every transaction: no snapshot needs it */
 
 		rs->lastRowDeleted = deleted;
 		return true;
@@ -3479,6 +3502,20 @@ PgColumnarReadSetEmitDeleted(PgColumnarReadState *readState, bool emit)
 	readState->emitDeleted = emit;
 }
 
+/*
+ * PgColumnarReadSetDeadHorizon
+ *		With PgColumnarReadSetEmitDeleted, leave out the deleted rows that are
+ *		deleted under `horizon` too (#1407). The index build passes a snapshot that
+ *		sees only transactions older than every running snapshot, so a row deleted
+ *		there is dead to all and needs no index entry, as heap's build skips a dead
+ *		tuple. Without it every deleted row was indexed, and REINDEX reclaimed none.
+ */
+void
+PgColumnarReadSetDeadHorizon(PgColumnarReadState *readState, Snapshot horizon)
+{
+	readState->deadHorizon = horizon;
+}
+
 bool
 PgColumnarReadLastRowDeleted(PgColumnarReadState *readState)
 {

src/columnar_reader.h

@@ -45,6 +45,7 @@ extern bool PgColumnarReadFoldColumn(PgColumnarReadState *readState, int attidx,
 								   int16 *attlen, const uint32 **vecRawLen);
 
 extern void PgColumnarReadSetEmitDeleted(PgColumnarReadState *readState, bool emit);
+extern void PgColumnarReadSetDeadHorizon(PgColumnarReadState *readState, Snapshot horizon);
 extern bool PgColumnarReadLastRowDeleted(PgColumnarReadState *readState);
 extern void PgColumnarReadSetProjection(PgColumnarReadState *readState,
 										Bitmapset *projectedColumns);

src/columnar_tableam.c

@@ -32,6 +32,7 @@
 #include "catalog/storage.h"
 #include "commands/defrem.h"
 #include "commands/extension.h"
+#include "storage/procarray.h"
 #include "commands/trigger.h"
 #include "catalog/dependency.h"
 #include "access/htup_details.h"
@@ -2419,6 +2420,8 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
 								IndexBuildCallback callback, void *callback_state,
 								TableScanDesc scan)
 {
+	SnapshotData deadHorizon;
+	Snapshot	deadHorizonReg;
 	PgColumnarReadState *readState;
 	bool		ownReadState;
 	Bitmapset  *projected;
@@ -2547,6 +2550,24 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
 	/* deleted rows too, flagged, so the build can index them as not alive (#1341) */
 	PgColumnarReadSetEmitDeleted(readState, true);
 
+	/*
+	 * BUT NOT A ROW DEAD TO EVERY TRANSACTION (#1407). The horizon sees exactly the
+	 * transactions committed before every running snapshot began, so a row deleted
+	 * under it is one no snapshot can see, and heap's build skips the like. With
+	 * every deleted row indexed, REINDEX -- documented as what reclaims stale
+	 * entries -- reclaimed none: 9 rows in 10 deleted, and the rebuilt index kept
+	 * its size where heap's shrank to a tenth. curcid 0 keeps this transaction's
+	 * own writes out of it; a row it deleted is not dead to all anyway.
+	 */
+	memset(&deadHorizon, 0, sizeof(deadHorizon));
+	deadHorizon.snapshot_type = SNAPSHOT_MVCC;
+	deadHorizon.xmin = GetOldestNonRemovableTransactionId(table_rel);
+	deadHorizon.xmax = deadHorizon.xmin;
+	deadHorizon.curcid = FirstCommandId;
+	/* registered: a visibility check asserts it (PG 18+), and the copy outlives this frame */
+	deadHorizonReg = RegisterSnapshot(&deadHorizon);
+	PgColumnarReadSetDeadHorizon(readState, deadHorizonReg);
+
 	/*
 	 * Say which branch ran and how wide the reader it produced will actually
 	 * read, so a test can assert the projection NARROWED rather than infer it
@@ -2624,6 +2645,7 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
 	 */
 	if (scan != NULL)
 		table_endscan(scan);
+	UnregisterSnapshot(deadHorizonReg);
 
 	return reltuples;
 }

test/alter_table_scans.sh

@@ -0,0 +1,82 @@
+#!/usr/bin/env bash
+#
+# ALTER TABLE forms that read every existing row through the access method (#1341):
+# SET NOT NULL, ADD PRIMARY KEY, VALIDATE CONSTRAINT, and the two that rewrite the
+# table, ALTER COLUMN TYPE and ADD COLUMN with a volatile default. No test ran them.
+#
+# A columnar table keeps a deleted row in its file until compaction, so the question
+# each form has to answer is the same: does it see the live rows, and only those?
+# Each form runs on a columnar table and on heap with the same rows. First a row
+# violates it, and both must refuse it with the same SQLSTATE. Then that row is
+# deleted, and both must accept it: a deleted NULL, duplicate or negative value
+# that the scan still saw would refuse it. Each rewrite must keep the live rows and
+# leave the deleted ones gone.
+#
+# Usage:  test/alter_table_scans.sh [PG_CONFIG]
+# Written fresh for pgColumnar.
+
+set -uo pipefail
+. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
+pgc_setup "${1:-/usr/local/pg17/bin/pg_config}"
+
+outcome() {	# outcome SQL -> ok, or the SQLSTATE the statement failed with
+	local out st
+	out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \
+		-X -qtA -v ON_ERROR_STOP=1 -v VERBOSITY=sqlstate -c "$1" 2>&1)"
+	st="$(sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' <<<"$out")"
+	echo "${st:-ok}"
+}
+both() {	# both SQL -> "<columnar outcome>|<heap outcome>", %t standing for the table
+	echo "$(outcome "${1//%t/at_pgcolumnar}")|$(outcome "${1//%t/at_heap}")"
+}
+answers() {	# answers SQL -> "<columnar answer>|<heap answer>", %t standing for the table
+	echo "$(q "${1//%t/at_pgcolumnar}" | tail -1)|$(q "${1//%t/at_heap}" | tail -1)"
+}
+
+# 3000 ordinary rows; then a NULL in v, a second id 1, and a negative v.
+for am in pgcolumnar heap; do
+	psql_run "CREATE TABLE at_$am (id int, v int, w text) USING $am;
+	          INSERT INTO at_$am SELECT g, g, 'x' FROM generate_series(1, 3000) g;
+	          INSERT INTO at_$am VALUES (5000, NULL, 'null'), (1, -1, 'dup');"
+done
+
+# ---- each form refuses the row that violates it ----------------------------------
+check "SET NOT NULL is refused over a NULL (23502), as on heap" \
+	"$(both "ALTER TABLE %t ALTER COLUMN v SET NOT NULL")" "23502|23502"
+check "ADD PRIMARY KEY is refused over a duplicate (23505), as on heap" \
+	"$(both "ALTER TABLE %t ADD PRIMARY KEY (id)")" "23505|23505"
+check "a CHECK added NOT VALID is accepted, as on heap" \
+	"$(both "ALTER TABLE %t ADD CONSTRAINT v_pos CHECK (v > 0) NOT VALID")" "ok|ok"
+check "and VALIDATE CONSTRAINT is refused over the row that breaks it (23514), as on heap" \
+	"$(both "ALTER TABLE %t VALIDATE CONSTRAINT v_pos")" "23514|23514"
+
+# ---- once those rows are deleted, each form accepts --------------------------------
+psql_run "DELETE FROM at_pgcolumnar WHERE v IS NULL OR v < 0; DELETE FROM at_heap WHERE v IS NULL OR v < 0;"
+check "the deleted rows are gone from both tables, which hold the same rows" \
+	"$(answers "SELECT count(*) || '/' || sum(v) FROM %t")" "3000/4501500|3000/4501500"
+check "SET NOT NULL succeeds once the NULL row is deleted, as on heap" \
+	"$(both "ALTER TABLE %t ALTER COLUMN v SET NOT NULL")" "ok|ok"
+check "ADD PRIMARY KEY succeeds once the duplicate is deleted, as on heap" \
+	"$(both "ALTER TABLE %t ADD PRIMARY KEY (id)")" "ok|ok"
+check "VALIDATE CONSTRAINT succeeds once the breaking row is deleted, as on heap" \
+	"$(both "ALTER TABLE %t VALIDATE CONSTRAINT v_pos")" "ok|ok"
+check "and the constraint is recorded as validated" \
+	"$(answers "SELECT convalidated FROM pg_constraint WHERE conrelid = '%t'::regclass AND conname = 'v_pos'")" "t|t"
+
+# ---- the two rewrites keep the live rows and only those ----------------------------
+check "ALTER COLUMN TYPE rewrites the table, as on heap" \
+	"$(both "ALTER TABLE %t ALTER COLUMN v TYPE bigint")" "ok|ok"
+check "and the rewrite kept the live rows and not the deleted ones" \
+	"$(answers "SELECT count(*) || '/' || sum(v) || '/' || (SELECT format_type(atttypid, NULL) FROM pg_attribute WHERE attrelid = '%t'::regclass AND attname = 'v') FROM %t")" "3000/4501500/bigint|3000/4501500/bigint"
+check "ADD COLUMN with a volatile default rewrites the table, as on heap" \
+	"$(both "ALTER TABLE %t ADD COLUMN z float8 DEFAULT random()")" "ok|ok"
+check "and that rewrite gave every live row a value and brought back no deleted one" \
+	"$(answers "SELECT count(*) || '/' || count(z) || '/' || count(*) FILTER (WHERE v IS NULL OR v < 0) FROM %t")" \
+	"3000/3000/0|3000/3000/0"
+IDX="SET enable_seqscan = off; SET enable_bitmapscan = off;"
+check "the lookup of id 1 is an index scan on the primary key" \
+	"$(for t in at_pgcolumnar at_heap; do q "$IDX EXPLAIN (COSTS OFF) SELECT count(*) FROM $t WHERE id = 1" | grep -c "Index.* Scan using ${t}_pkey"; done | paste -sd'|')" "1|1"
+check "the primary key still finds id 1 once, through the index, after both rewrites" \
+	"$(answers "$IDX SELECT count(*) || '/' || min(w) FROM %t WHERE id = 1")" "1/x|1/x"
+
+pgc_summary

test/am_contracts.sh

@@ -280,17 +280,41 @@ check "an insert of a key an uncommitted UPDATE moved away fails when the update
 # as on heap, which indexes rows not yet dead to every snapshot.
 psql_run "CREATE TABLE ib (id int, v int) USING pgcolumnar;"
 psql_run "INSERT INTO ib SELECT g, g FROM generate_series(1, 10000) g;"
+# ORDERED BY EVENTS, NOT SLEEPS. Session 1 used to sleep four seconds and assume the
+# DELETE and the index build fitted inside them; on a loaded box they did not, the
+# plan was taken before the index existed, and the premise read 0 (PG16, 2026-10-15).
+# Now a gate session holds an advisory lock. Session 1 takes its snapshot, names
+# itself, and queues for that lock; the script waits for the name, deletes, builds,
+# and only then ends the gate, so session 1 plans after the index exists.
 _ib_out="$PGC_WORKDIR/ib_rr.out"
+ib_wait_for() {	# ib_wait_for APPLICATION_NAME -> its pid, once it has named itself
+	local i pid=""
+	for i in $(seq 1 300); do
+		pid="$(q "SELECT pid FROM pg_stat_activity WHERE application_name = '$1'")"
+		[ -n "$pid" ] && break
+		sleep 0.1
+	done
+	echo "$pid"
+}
+env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \
+	-c "SELECT pg_advisory_lock(4242); SET application_name = 'ib_gate'; SELECT pg_sleep(600);" >/dev/null 2>&1 &
+_ib_gate=$!
+_ib_gate_pid="$(ib_wait_for ib_gate)"
 env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq -v ON_ERROR_STOP=1 \
-	-c "BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM ib; SELECT pg_sleep(4);
+	-c "BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM ib; SET application_name = 'ib_snap';
+	    SELECT pg_advisory_lock_shared(4242);
 	    SET LOCAL enable_seqscan = off; SET LOCAL enable_bitmapscan = off;
 	    EXPLAIN (COSTS OFF) SELECT count(*) FROM ib WHERE id = 4242;
 	    SELECT 'rows:' || count(*) FROM ib WHERE id = 4242; COMMIT;" > "$_ib_out" 2>&1 &
 _ib_s1=$!
-sleep 1
+_ib_snap_pid="$(ib_wait_for ib_snap)"
 psql_run "DELETE FROM ib WHERE id = 4242;"
 psql_run "CREATE INDEX ib_id ON ib (id);"
+q "SELECT pg_terminate_backend(${_ib_gate_pid:-0})" >/dev/null
+wait "$_ib_gate" 2>/dev/null
 wait "$_ib_s1"
+check "premise: the older snapshot was taken before the delete" \
+	"$([ -n "$_ib_gate_pid" ] && [ -n "$_ib_snap_pid" ] && head -1 "$_ib_out" || echo "no (gate ${_ib_gate_pid:-none}, snapshot ${_ib_snap_pid:-none})")" "10000"
 check "premise: the index exists and is valid" \
 	"$(q "SELECT indisvalid FROM pg_index WHERE indexrelid = 'ib_id'::regclass")" "t"
 check "premise: the older snapshot's lookup is an index scan" \
@@ -309,4 +333,22 @@ psql_run "INSERT INTO ibu VALUES (5);"
 check "a unique index builds over a key whose other holder is a deleted row" \
 	"$(state "CREATE UNIQUE INDEX ibu_k ON ibu (k);")|$(q "SET enable_seqscan = off; SELECT count(*) FROM ibu WHERE k = 5;" | tail -1)" "ok|1"
 
+# But only rows some snapshot can still see (#1407). Every deleted row went in, so
+# REINDEX, documented as what reclaims stale entries, reclaimed none: 9 rows in 10
+# deleted, and the rebuilt index kept its size where heap's shrank to a tenth.
+for am in pgcolumnar heap; do
+	psql_run "CREATE TABLE ix_$am (a int) USING $am; INSERT INTO ix_$am SELECT g FROM generate_series(1, 100000) g;
+	          CREATE INDEX ix_${am}_a ON ix_$am (a); DELETE FROM ix_$am WHERE a % 10 <> 0;"
+done
+ix_reclaim() {	# ix_reclaim AM -> reclaimed, when REINDEX takes the index under half its size
+	local before after
+	before="$(q "SELECT pg_relation_size('ix_${1}_a')")"
+	psql_run "REINDEX INDEX ix_${1}_a;"
+	after="$(q "SELECT pg_relation_size('ix_${1}_a')")"
+	[ -n "$before" ] && [ -n "$after" ] && [ "$after" -lt $((before / 2)) ] && echo reclaimed || echo "kept:$before->$after"
+}
+check "REINDEX after a committed delete of most rows reclaims their entries, as heap's does" \
+	"$(ix_reclaim pgcolumnar)|$(ix_reclaim heap)|$(q "SET enable_seqscan = off; SELECT count(*) FROM ix_pgcolumnar WHERE a > 0;" | tail -1)" \
+	"reclaimed|reclaimed|10000"
+
 pgc_summary