diff --git a/CHANGELOG.md b/CHANGELOG.md index 333c91c8..11e2a549 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,42 @@ true until the next version shipped. ### Fixed +- `pgcolumnar.expire()` no longer drops live rows, and every path that renumbers + live rows now clears the visibility map (#403). + + **Three separate ways to lose data or read a row that is gone.** + + A group holding a row with a `NULL` retention value could be retired, taking + live rows with it. `expire` now keeps any group whose retention column has a + LIVE `NULL`, because a `NULL` has no age and the group cannot be known to be + wholly expired. + + Live is the operative word. The zone map's null count is recorded when the + group is written and never revised, so it still counts rows a later `DELETE` + marked. Reading it as the live count keeps a group whose every live row is + past retention and whose `NULL` rows have all been deleted, and keeps it + permanently, because nothing rewrites a zone map on delete. That trades data + loss for silent over-retention. `expire` now checks the live rows, and only + for a group that has both recorded `NULL`s and deletes: with no delete vector + the recorded count is still exact, so the metadata-only path is unchanged and + `expire` still reads nothing. + + A negative `ttl_interval` put the cutoff in the future, so `expire` retired + groups that were entirely inside their retention. `set_options` range-checked + every other option it accepts and not this one. Zero and negative intervals + now raise `22023`. + + Retiring a group leaves its old row numbers in the visibility map, so an + index-only scan answers from the index for a row group that is gone. `expire` + cleared them; `pgcolumnar.recluster()` and the partial-group rewrite behind + `pgcolumnar.compact_rewrite()` did not, and both renumber live rows through + the same retire. All three clear now. The rule is that visibility-map bits go + wherever row numbers are reassigned, not only where rows expire. + + `docs/sql-reference.md` gains the accepted range for `ttl_interval` and the + `NULL` rule, which is stronger than the straddling behaviour the page already + described: a straddling group is released once its newest row ages past the + cutoff, and a group holding a `NULL` never is. - Arrow import reads the temporal unit and carrier width the file declares, rather than assuming the ones our own exporter writes (#864, #865). diff --git a/docs/sql-reference.md b/docs/sql-reference.md index 5b0d26d9..1ba0d00f 100644 --- a/docs/sql-reference.md +++ b/docs/sql-reference.md @@ -38,6 +38,10 @@ only by [`pgcolumnar.expire`](#pgcolumnarexpiretablename-regclass-returns-bigint which you run yourself. Declaring a retention does not delete anything on its own. Both are needed: either one alone means no retention. +`ttl_interval` must be a positive interval. Zero and negative intervals raise +`22023`. A negative interval would put the cutoff in the future, so `expire` +would drop rows that are still inside their retention. + ```sql SELECT pgcolumnar.set_options('events', sort_by => ARRAY['customer_id','ts']); SELECT pgcolumnar.reset_options('events', sort_by => true); -- clear it @@ -187,6 +191,17 @@ The retention column must be `timestamp` or `timestamptz`. The table must have both `ttl_column` and `ttl_interval` declared, or the function raises an error rather than reporting that it did nothing. +`expire` works on whole row groups. It never reads or rewrites them, so it drops +a group only when every row in it is past the retention. A group that straddles +the cutoff stays whole, and rows older than the retention survive in it. + +One live `NULL` in the retention column pins its entire row group. A `NULL` has +no age, so the group cannot be known to be wholly expired. Deleting the `NULL` +rows releases the group, and the next `expire` can drop it. This is stronger than the straddling rule +above. A straddling group is released once its newest row ages past the cutoff. +A group holding a `NULL` never is. Keep the retention column `NOT NULL` if you +want `expire` to reclaim the space. + ```sql SELECT pgcolumnar.set_options('events', ttl_column => 'ts', ttl_interval => '90 days'); diff --git a/pgcolumnar--1.0-alpha3.sql b/pgcolumnar--1.0-alpha3.sql index 56596d89..f72a2ea8 100644 --- a/pgcolumnar--1.0-alpha3.sql +++ b/pgcolumnar--1.0-alpha3.sql @@ -497,6 +497,30 @@ BEGIN RAISE EXCEPTION 'compression_level must be between 1 and 22'; END IF; + /* + * A negative retention puts the cutoff in the FUTURE, so expire finds + * `maximum < cutoff` true for groups that are entirely inside their + * retention and retires them. That drops live rows, which is the failure + * this option exists to prevent. Every other option here is range-checked + * and this one was not. + * + * Zero is refused too. It is not a data-loss shape -- the cutoff is now, so + * only groups already wholly in the past go -- but "expire everything older + * than nothing" has no reading a caller means on purpose, and accepting it + * silently makes a typo indistinguishable from an instruction. + * + * ERRCODE is explicit for the reason the relkind guard above gives: this + * tree's suites assert SQLSTATE rather than message text, and plpgsql would + * otherwise default to P0001. + */ + IF ttl_interval IS NOT NULL AND ttl_interval <= interval '0' THEN + RAISE EXCEPTION 'ttl_interval must be a positive interval, not %', ttl_interval + USING ERRCODE = 'invalid_parameter_value', + HINT = 'A negative retention puts the cutoff in the future, ' + 'so pgcolumnar.expire() would retire groups whose rows are ' + 'still within their retention.'; + END IF; + /* * sort_by declares the physical sort key applied by vacuum_sorted() with no * explicit columns (#288). This is a cheap early check only: each named diff --git a/src/columnar.h b/src/columnar.h index 9ee40969..21d22f41 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -385,6 +385,8 @@ extern uint64 PgColumnarItemPointerToRowNumber(ItemPointer tid); * visibility map for index-only scans (pgcolumnar_visibilitymap.c, gap 28) * ------------------------------------------------------------------------- */ extern void PgColumnarVMClearForRow(Relation rel, uint64 rowNumber); +extern void PgColumnarVMClearForRowRange(Relation rel, uint64 firstRowNumber, + uint64 rowCount); extern uint64 PgColumnarVMSetVisibleForRelation(Relation rel); /* index maintenance for callers that insert rows without an executor (#153) */ diff --git a/src/columnar_vacuum.c b/src/columnar_vacuum.c index b041973b..a0d737f0 100644 --- a/src/columnar_vacuum.c +++ b/src/columnar_vacuum.c @@ -202,6 +202,8 @@ PgColumnarRequireTableOwnerByOid(Oid relid) /* Z-order helpers (defined later, used by the online recluster below) */ static bool cluster_type_supported(Oid typid); +static bool group_has_live_null(Relation rel, NativeRowGroupMetadata *rg, + AttrNumber attno); static bytea *cluster_zorder_key(Datum *values, bool *isnull, AttrNumber *atts, int ncols, TupleDesc tupdesc); /* Names an ordering rewrite records as its key (defined later, #415) */ @@ -221,6 +223,29 @@ uint64_cmp(const void *a, const void *b) return (x < y) ? -1 : (x > y) ? 1 : 0; } +/* + * A retired group and the row numbers it held. Recluster needs both: the number + * to lock and retire in ascending order, and the range to clear from the + * visibility map, because those row numbers are being reassigned. Carried in + * one struct rather than parallel arrays so the sort below cannot separate a + * group from its range. + */ +typedef struct RetiredGroup +{ + uint64 groupNumber; + uint64 firstRowNumber; + uint64 rowCount; +} RetiredGroup; + +static int +retired_group_cmp(const void *a, const void *b) +{ + uint64 x = ((const RetiredGroup *) a)->groupNumber; + uint64 y = ((const RetiredGroup *) b)->groupNumber; + + return (x < y) ? -1 : (x > y) ? 1 : 0; +} + /* ------------------------------------------------------------------------- * Online rewrite of partially-deleted row groups (Phase F3b) * @@ -311,7 +336,12 @@ rewrite_one_group(Relation rel, PgColumnarIndexInsertState *ris, uint64 storageI PgColumnarFlushWriteStateForRelation(relid); /* atomically (same transaction) the new group is now in the catalog; drop the - * old one. Heap MVCC keeps the old group readable to older snapshots. */ + * old one. Heap MVCC keeps the old group readable to older snapshots. + * + * The rows just moved carry NEW row numbers, so the old numbers' visibility + * map bits must go with the group. Without this an index-only scan answers + * from the index for a TID whose group no longer exists. */ + PgColumnarVMClearForRowRange(rel, firstRow, rowCount); PgColumnarRetireGroup(storageId, groupNumber); PopActiveSnapshot(); @@ -539,7 +569,7 @@ pgcolumnar_recluster_online(Relation rel, int ncols, AttrNumber *atts) Snapshot listSnap; List *rgList; ListCell *lc; - uint64 *oldGroups; + RetiredGroup *oldGroups; int nGroups = 0; int i; Snapshot snap; @@ -569,12 +599,16 @@ pgcolumnar_recluster_online(Relation rel, int ncols, AttrNumber *atts) /* capture the current groups (retired at the end, after the new ones exist) */ listSnap = RegisterSnapshot(GetLatestSnapshot()); rgList = PgColumnarReadRowGroupList(storageId, listSnap); - oldGroups = palloc(sizeof(uint64) * (list_length(rgList) > 0 ? list_length(rgList) : 1)); + oldGroups = palloc(sizeof(RetiredGroup) * + (list_length(rgList) > 0 ? list_length(rgList) : 1)); foreach(lc, rgList) { NativeRowGroupMetadata *rg = (NativeRowGroupMetadata *) lfirst(lc); - oldGroups[nGroups++] = rg->groupNumber; + oldGroups[nGroups].groupNumber = rg->groupNumber; + oldGroups[nGroups].firstRowNumber = rg->firstRowNumber; + oldGroups[nGroups].rowCount = rg->rowCount; + nGroups++; } UnregisterSnapshot(listSnap); @@ -588,7 +622,7 @@ pgcolumnar_recluster_online(Relation rel, int ncols, AttrNumber *atts) errhint("Use pgcolumnar.cluster() for a one-shot reorg of a very large table."))); /* lock every group in ascending order (deadlock-safe), held to commit */ - qsort(oldGroups, nGroups, sizeof(uint64), uint64_cmp); + qsort(oldGroups, nGroups, sizeof(RetiredGroup), retired_group_cmp); /* * Self-gate (#415): if the whole live relation is already the Z-order run @@ -627,8 +661,8 @@ pgcolumnar_recluster_online(Relation rel, int ncols, AttrNumber *atts) } } if (sameKey && sfrom >= 0 && sthrough >= 0 && - (int64) oldGroups[0] >= sfrom && - (int64) oldGroups[nGroups - 1] <= sthrough) + (int64) oldGroups[0].groupNumber >= sfrom && + (int64) oldGroups[nGroups - 1].groupNumber <= sthrough) { pfree(oldGroups); /* no active snapshot pushed yet at this point -- see below */ @@ -637,7 +671,7 @@ pgcolumnar_recluster_online(Relation rel, int ncols, AttrNumber *atts) } for (i = 0; i < nGroups; i++) - PgColumnarLockChunkGroup(storageId, oldGroups[i]); + PgColumnarLockChunkGroup(storageId, oldGroups[i].groupNumber); /* read all live rows into a Morton-keyed tuplesort (as in eager cluster). * Register the snapshot (not just push active) so the catalog snapshot copies @@ -715,9 +749,21 @@ pgcolumnar_recluster_online(Relation rel, int ncols, AttrNumber *atts) tuplesort_end(tsort); ExecDropSingleTupleTableSlot(augSlot); - /* retire the old groups; heap MVCC keeps them readable to older snapshots */ + /* + * Retire the old groups; heap MVCC keeps them readable to older snapshots. + * + * Clear the visibility map over the row numbers each retired group held. + * Recluster reassigns those rows fresh numbers, so an index-only scan that + * answers from the index for an old TID would answer for a group that is + * gone. That is the same rule expire follows, and it holds wherever LIVE + * rows are renumbered rather than only where they expire. + */ for (i = 0; i < nGroups; i++) - PgColumnarRetireGroup(storageId, oldGroups[i]); + { + PgColumnarVMClearForRowRange(rel, oldGroups[i].firstRowNumber, + oldGroups[i].rowCount); + PgColumnarRetireGroup(storageId, oldGroups[i].groupNumber); + } /* record how far the reordered run reaches (#311) and BY WHAT (#415) */ record_online_sorted_extent(rel, storageId, writeState, stripeMark, @@ -2039,6 +2085,17 @@ pgcolumnar_cluster(PG_FUNCTION_ARGS) * by group rather than by row, and it is the safe direction: the * alternative drops rows that are still inside the retention. * + * A NULL in the retention column is the same kind of straddle. The zone + * map's maximum covers only the non-NULL timestamps, so a group whose + * timestamps are all past the cutoff can still hold rows whose retention + * is unknown. Dropping it would delete those rows. + * + * Retiring a live group also clears the visibility-map bits covering its + * row numbers. VACUUM may already have marked the group all-visible, and + * an index-only scan would then return the expired keys from the index + * without fetching. A fetch would correctly fail once the catalog rows + * are gone; skipping the fetch is what made the ghosts visible. + * * This is called by name and never runs on its own. It deletes rows, and an * operation a user runs for maintenance must not do that silently, so it is * not wired into vacuum, compact or autovacuum. @@ -2057,6 +2114,7 @@ pgcolumnar_expire(PG_FUNCTION_ARGS) TypeCacheEntry *tce; uint64 storageId; List *groups; + bool anyDeletes; ListCell *lc; int64 retired = 0; int i; @@ -2144,6 +2202,60 @@ pgcolumnar_expire(PG_FUNCTION_ARGS) storageId = PgColumnarStorageId(rel); groups = PgColumnarReadRowGroupList(storageId, GetActiveSnapshot()); + /* + * One probe for the whole storage, not one per group. With no delete vector + * anywhere, a zone map's write-time nullCount is still exact, so expire + * keeps its metadata-only path and reads nothing at all -- which is what it + * promises. Only a table that has deletes can have a stale nullCount, and + * only groups in such a table are looked at. + * + * This is COARSE on purpose, and what it costs is recorded here so that + * changing it later is a decision rather than a rediscovery. A per-group + * deleted count would skip the read for a null-bearing group carrying no + * deletes; this probe makes one delete anywhere in the storage send every + * such group down the read path. + * + * Measured on PG 17.10 at 10000 rows a group, six repetitions, arm order + * alternated, every point asserting both arms retired all N groups so they + * differ only in the path: + * + * 5 groups read 3-7 ms metadata 1-2 ms + * 40 groups read 21-37 ms metadata 2-5 ms + * + * THOSE ARE TOTALS, AND THEY ARE NOT A RATE. Do not divide by the group + * count and multiply back up. Two independent sweeps on this same box, at + * this same geometry, agree on the totals and disagree on the decomposition: + * fitting a + b*groups gives roughly 0.7 ms fixed with 0.59 ms per group + * from one, and 2.6 ms fixed with 0.27 ms per group from the other. Neither + * dataset determines which, because the repetition spread (21 to 37 ms at 40 + * groups) is comparable to the difference being fitted. A reader who took + * 0.5 ms per group and multiplied by a table of several thousand groups + * would get seconds, and nothing here supports that. + * + * What IS supported: at these sizes the whole thing is milliseconds, and the + * read arm is under 40 ms for a 400,000-row table. Nothing has been measured + * at the shipped stripe_row_limit of 150000, where a group holds fifteen + * times these rows. + * + * AND THE INSTRUMENT IS WALL CLOCK ON A SHARED, CONTENDED HOST, which is the + * likeliest reason the two sweeps decomposed differently at all: the same 40 + * groups timed 21-37 ms in one and 12-21 ms in the other, taken while a + * second tenant was building and running suites on the same eight cores. + * That difference is larger than the effect either fit was resolving. These + * numbers bound the MAGNITUDE and cannot support a shape. If the shape ever + * matters, measure instructions retired by the backend (perf stat -p on the + * backend pid) rather than elapsed time: contention moves the clock and does + * not move the instruction count. + * + * It is kept because of what kind of path this is. relation_estimate_size + * runs on every plan of every query, and a per-group fold there was worth + * removing. expire is a maintenance function called by name, where a few + * milliseconds is a different class of problem -- and the per-group count is + * only exposed in a header by a separate PR, so using it would couple this + * fix to that one landing first. + */ + anyDeletes = PgColumnarStorageHasDeleteVector(storageId, GetActiveSnapshot()); + foreach(lc, groups) { NativeRowGroupMetadata *rg = (NativeRowGroupMetadata *) lfirst(lc); @@ -2163,6 +2275,28 @@ pgcolumnar_expire(PG_FUNCTION_ARGS) if (z == NULL || !z->hasMinMax) continue; + /* + * Keep any group that holds a LIVE NULL in the retention column. The + * maximum cannot speak for those rows, and treating "every timestamp we + * can see is expired" as "the group is expired" drops them. + * + * z->nullCount is recorded at WRITE time and never revised, so it still + * counts rows a later DELETE marked. Reading it as the live count keeps + * a group whose every live row is past retention and whose NULL rows + * have all been deleted -- and keeps it FOREVER, because nothing + * rewrites a zone map on delete. That trades data loss for permanent + * over-retention, which is quieter and not better. + * + * With no deletes the two counts agree, so the metadata answer stands + * and expire still reads nothing. Only a group with recorded NULLs AND + * deletes is ambiguous, and only that group is looked at. + */ + if (z->nullCount > 0) + { + if (!anyDeletes || group_has_live_null(rel, rg, attno)) + continue; + } + cur = (char *) z->maximum; maxv = PgColumnarDecodeValue(att, &cur, z->maximum + z->maximumLen, CurrentMemoryContext); @@ -2171,6 +2305,7 @@ pgcolumnar_expire(PG_FUNCTION_ARGS) att->attcollation, maxv, cutoff)); if (c < 0) { + PgColumnarVMClearForRowRange(rel, rg->firstRowNumber, rg->rowCount); PgColumnarRetireGroup(storageId, rg->groupNumber); retired++; } @@ -2182,6 +2317,47 @@ pgcolumnar_expire(PG_FUNCTION_ARGS) PG_RETURN_INT64(retired); } +/* + * group_has_live_null + * Does any LIVE row of this group hold a NULL in the retention column? + * + * The zone map's nullCount is recorded at WRITE time and is never revised, so it + * still counts rows that a later DELETE marked. It answers "were any NULLs ever + * written here", which is not the question expire has to ask. + * + * Nothing in the metadata says WHICH rows were null, so when the group has both + * recorded NULLs and deletes the only way to tell is to look. This reads one + * column of one group through the ordinary reader, which merges the delete + * vector, so every row it yields is live. + */ +static bool +group_has_live_null(Relation rel, NativeRowGroupMetadata *rg, AttrNumber attno) +{ + TupleDesc tupdesc = RelationGetDescr(rel); + Datum *values = palloc(sizeof(Datum) * tupdesc->natts); + bool *isnull = palloc(sizeof(bool) * tupdesc->natts); + PgColumnarReadState *rs; + uint64 rowNumber; + bool found = false; + + rs = PgColumnarBeginRead(rel, GetActiveSnapshot(), NULL, NULL, 0, NULL); + PgColumnarReadRestrictToGroups(rs, &rg->groupNumber, 1); + while (PgColumnarReadNextRow(rs, values, isnull, &rowNumber)) + { + CHECK_FOR_INTERRUPTS(); + if (isnull[attno - 1]) + { + found = true; + break; + } + } + PgColumnarEndRead(rs); + + pfree(values); + pfree(isnull); + return found; +} + Datum pgcolumnar_compact(PG_FUNCTION_ARGS) { diff --git a/src/columnar_visibilitymap.c b/src/columnar_visibilitymap.c index 2046a535..e2aa4153 100644 --- a/src/columnar_visibilitymap.c +++ b/src/columnar_visibilitymap.c @@ -169,6 +169,31 @@ PgColumnarVMClearForRow(Relation rel, uint64 rowNumber) PgColumnarVMClearVisible(rel, blk); } +/* + * PgColumnarVMClearForRowRange + * Clear the all-visible bit for every synthetic block covering + * [firstRowNumber, firstRowNumber + rowCount). Used when a whole live + * row group is retired (pgcolumnar.expire) so an index-only scan cannot + * keep answering from the VM after the group's catalog rows are gone. + */ +void +PgColumnarVMClearForRowRange(Relation rel, uint64 firstRowNumber, uint64 rowCount) +{ + uint64 last; + BlockNumber b0; + BlockNumber b1; + BlockNumber blk; + + if (rowCount == 0) + return; + + last = firstRowNumber + rowCount - 1; + b0 = (BlockNumber) (firstRowNumber / COLUMNAR_VALID_ITEMPOINTER_OFFSETS); + b1 = (BlockNumber) (last / COLUMNAR_VALID_ITEMPOINTER_OFFSETS); + for (blk = b0; blk <= b1; blk++) + PgColumnarVMClearVisible(rel, blk); +} + /* * PgColumnarVMIsVisible * True if `blk` is marked all-visible in the VM fork. Thin wrapper over the diff --git a/test/ttl_expire.sh b/test/ttl_expire.sh index 271f81f8..15919b7b 100755 --- a/test/ttl_expire.sh +++ b/test/ttl_expire.sh @@ -25,6 +25,15 @@ # data disappears would pass just as well on an implementation that dropped # everything. # +# Two more ways a "the maximum is expired" reading is not "every row is +# expired": +# * a NULL in the retention column. The zone map's max covers only non-NULL +# timestamps, so a group of old timestamps plus NULLs looks fully expired +# and would drop the NULLs. +# * an index-only scan after VACUUM. expire retires live groups without +# going through the delete vector, so the VM bits VACUUM set stay on and +# the scan answers from the index without fetching the (now missing) group. +# # Usage: test/ttl_expire.sh [PG_CONFIG] # Written fresh for pgColumnar. @@ -127,4 +136,194 @@ ERR="$(q "SELECT pgcolumnar.expire('ttl_none')")" check "a table with no declared retention is an error, not a silent success" \ "$(grep -qiE 'ERROR|no retention|ttl' <<<"$ERR" && echo "refused" || echo "ACCEPTED ($ERR)")" "refused" +# ---- NULL in the retention column is a straddle, not an expiry ------------ +psql_run "CREATE TABLE ttl_null (id int, ts timestamptz, v text) USING pgcolumnar;" +psql_run "INSERT INTO ttl_null + SELECT g, + CASE WHEN g <= 10 THEN NULL + ELSE now() - interval '10 days' END, + 'v'||g + FROM generate_series(1,1000) g;" +psql_run "SELECT pgcolumnar.set_options('ttl_null', ttl_column => 'ts', + ttl_interval => '3 days');" +NULL_BEFORE="$(q "SELECT count(*) FROM ttl_null WHERE ts IS NULL")" +check "premise: the table holds NULL retention rows at all" "$NULL_BEFORE" "10" +# The arm below is about a GROUP that mixes expired rows with NULL ones. A +# table-level count cannot see a row group, and passes just as readily on a +# fixture where the NULL rows sit in a group of their own -- which is the +# arrangement the premise exists to exclude. Count the groups that hold both. +check "premise: and the SAME row group holds expired rows and NULL ones" \ + "$(q "SELECT count(*) FROM ( + SELECT z.group_number + FROM pgcolumnar.zone_map z + JOIN pgcolumnar.storage s ON s.storage_id = z.storage_id + WHERE s.relation_oid = 'ttl_null'::regclass + AND z.null_count > 0 + GROUP BY z.group_number) g")" "1" +NULL_RETIRED="$(q "SELECT pgcolumnar.expire('ttl_null')")" +NULL_AFTER="$(q "SELECT count(*) FROM ttl_null WHERE ts IS NULL")" +NULL_ROWS="$(q "SELECT count(*) FROM ttl_null")" +check "expire does not retire a group that still holds NULL retention rows" \ + "$NULL_RETIRED" "0" +check "and those NULL rows are still there" "$NULL_AFTER" "10" +check "and the expired timestamps sharing the group were kept with them" \ + "$NULL_ROWS" "1000" + +# ---- index-only scan must not return rows expire already retired ---------- +psql_run "CREATE TABLE ttl_ios (id int, ts timestamptz) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.set_options('ttl_ios', stripe_row_limit => 16384);" +psql_run "INSERT INTO ttl_ios SELECT g, now() - interval '10 days' + FROM generate_series(1,8000) g;" +psql_run "CREATE INDEX ttl_ios_id ON ttl_ios (id);" +psql_run "ALTER DATABASE $PGC_DB SET pgcolumnar.enable_index_only_scan = on;" +psql_run "ALTER DATABASE $PGC_DB SET pgcolumnar.enable_custom_scan = off;" +psql_run "ALTER DATABASE $PGC_DB SET enable_seqscan = off;" +psql_run "ALTER DATABASE $PGC_DB SET enable_bitmapscan = off;" +psql_run "VACUUM ttl_ios;" +psql_run "SELECT pgcolumnar.set_options('ttl_ios', ttl_column => 'ts', + ttl_interval => '3 days');" +ios_plan_before="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" \ + -U postgres -d "$PGC_DB" -Atq -c \ + "EXPLAIN (COSTS OFF) SELECT id FROM ttl_ios WHERE id BETWEEN 1 AND 8000;")" +check "premise: an index-only scan is chosen on the all-visible table" \ + "$(printf '%s' "$ios_plan_before" | grep -c 'Index Only Scan')" "1" +# The plan shape is decided by pg_class.relallvisible and by enable_seqscan and +# enable_bitmapscan being off -- NOT by the visibility-map bit the fix clears. +# Stop PgColumnarVMSetVisibleForRelation writing bits and the plan is unchanged, +# so the arm above cannot fail for the thing under test. Assert the bit itself. +check "premise: and VACUUM really did set visibility-map bits to clear" \ + "$(q "SELECT CASE WHEN relallvisible > 0 THEN 'set' ELSE 'none' END + FROM pg_class WHERE oid = 'ttl_ios'::regclass")" "set" +IOS_VM_BEFORE="$(q "SELECT relallvisible FROM pg_class WHERE oid = 'ttl_ios'::regclass")" +IOS_RETIRED="$(q "SELECT pgcolumnar.expire('ttl_ios')")" +check "expire retires the all-visible expired group" \ + "$([ "${IOS_RETIRED:-0}" -gt 0 ] && echo retired || echo "RETIRED NOTHING ($IOS_RETIRED)")" \ + "retired" +# Seqscan is the catalog truth: the group is gone. +SEQ_AFTER="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" \ + -U postgres -d "$PGC_DB" -Atq -c \ + "SET enable_seqscan = on; SET pgcolumnar.enable_custom_scan = on; + SELECT count(*) FROM ttl_ios;")" +check "seqscan agrees the expired rows are gone" "$(printf '%s' "$SEQ_AFTER" | tail -1)" "0" +ios_plan_after="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" \ + -U postgres -d "$PGC_DB" -Atq -c \ + "EXPLAIN (COSTS OFF) SELECT id FROM ttl_ios WHERE id BETWEEN 1 AND 8000;")" +check "the index-only scan is still the plan after expire" \ + "$(printf '%s' "$ios_plan_after" | grep -c 'Index Only Scan')" "1" +IOS_AFTER="$(q "SELECT count(*) FROM ttl_ios WHERE id BETWEEN 1 AND 8000")" +check "index-only scan does not return rows expire already retired" "$IOS_AFTER" "0" + +# NOT asserted here: that the visibility-map bits were CLEARED, as opposed to +# the consequence above. I tried and the arm was wrong -- pg_class.relallvisible +# is a statistic that VACUUM refreshes, and clearing a VM bit does not touch it, +# so the arm read "still 2 of 2" on a tree where the clear demonstrably works. +# Reading the fork itself needs pg_visibility, which is not built in this +# environment (no pg_visibility.control under the prefix's extension directory). +# +# So the clear is asserted through its consequence, with the premise above +# pinning the thing that was previously assumed: that VACUUM really did set bits +# for this table. That was the reviewer's ask, and it is what makes the +# index-only arm able to fail for the reason it names. + +# ---- a negative retention must be refused, not applied ---------------------- +# +# A negative ttl_interval puts the cutoff in the FUTURE, so expire finds +# `maximum < cutoff` true for groups entirely inside their retention and retires +# them: live rows dropped, which is the failure this suite is named for. +# SQLSTATE, not message text -- 22023 comes from the range check, while a +# missing function would be 42883 and a non-owner 42501. +ttl_state() { + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -v ON_ERROR_STOP=1 -Atq -v VERBOSITY=sqlstate -c "$1" 2>&1 \ + | sed -n 's/^ERROR: \([0-9A-Z]\{5\}\).*/\1/p' | head -1 +} +psql_run "CREATE TABLE ttl_neg (id int, ts timestamptz) USING pgcolumnar;" +psql_run "INSERT INTO ttl_neg SELECT g, now() FROM generate_series(1,100) g;" +check "a negative ttl_interval is refused with 22023" \ + "$(ttl_state "SELECT pgcolumnar.set_options('ttl_neg', ttl_column => 'ts', + ttl_interval => '-3 days');")" "22023" +check "and a zero ttl_interval is refused too" \ + "$(ttl_state "SELECT pgcolumnar.set_options('ttl_neg', ttl_column => 'ts', + ttl_interval => '0 seconds');")" "22023" +# Control: without this pair, a guard that refused EVERY interval would look +# identical to one that refuses only the dangerous ones. +check "control: and a positive one is still accepted" \ + "$(ttl_state "SELECT pgcolumnar.set_options('ttl_neg', ttl_column => 'ts', + ttl_interval => '3 days');")" "" +check "control: and the rows are all still there" \ + "$(q 'SELECT count(*) FROM ttl_neg')" "100" + +# ---- a NULL that has been DELETED must stop pinning its group --------------- +# +# z->nullCount is recorded at WRITE time and never revised, so it still counts +# rows a later DELETE marked. Reading it as the live count keeps a group whose +# every live row is past retention and whose NULL rows have all been deleted -- +# and keeps it FOREVER, because nothing rewrites a zone map on delete. That +# trades data loss for permanent over-retention, which is quieter and not +# better. +# +# Measured on both trees before the guard read a live-row property: +# +# main 53224e4 expire 1, rows left 0 (right, but by dropping the +# live NULL rows in arm L too) +# the null-count guard expire 0, rows left 810 (REFUSED, and would never +# retire this group again) +# +# ONE INSERT statement, not two. Two statements flush two row groups, the NULLs +# never share a group with the rows under test, and every arm below answers a +# question nobody asked. The group count is asserted rather than assumed for +# exactly that reason. + +psql_run "CREATE TABLE ttl_dn (id int, ts timestamptz, v text) USING pgcolumnar;" +psql_run "INSERT INTO ttl_dn + SELECT g, + CASE WHEN g % 10 = 0 THEN NULL + ELSE now() - interval '400 days' END, + 'v' || g + FROM generate_series(1,900) g;" +psql_run "SELECT pgcolumnar.set_options('ttl_dn', ttl_column => 'ts', + ttl_interval => '90 days');" + +check "premise: the fixture is ONE row group, so the NULLs share it" \ + "$(q "SELECT count(*) FROM pgcolumnar.row_group rg + JOIN pgcolumnar.storage s ON s.storage_id = rg.storage_id + WHERE s.relation_oid = 'ttl_dn'::regclass")" "1" + +# Delete every NULL row. The zone map is not rewritten, so nullCount still +# counts them -- which is the whole point. +psql_run "DELETE FROM ttl_dn WHERE ts IS NULL;" + +check "premise: no LIVE row holds a NULL retention value any more" \ + "$(q 'SELECT count(*) FROM ttl_dn WHERE ts IS NULL')" "0" +check "premise: but the zone map still records the deleted NULLs" \ + "$(q "SELECT CASE WHEN sum(z.null_count) > 0 THEN 'still recorded' ELSE 'gone' END + FROM pgcolumnar.zone_map z + JOIN pgcolumnar.storage s ON s.storage_id = z.storage_id + WHERE s.relation_oid = 'ttl_dn'::regclass")" "still recorded" +check "premise: and every live row is past the retention" \ + "$(q "SELECT count(*) FROM ttl_dn WHERE ts >= now() - interval '90 days'")" "0" + +DN_RETIRED="$(q "SELECT pgcolumnar.expire('ttl_dn')")" +check "a group whose only NULLs have been deleted is retired" "$DN_RETIRED" "1" +check "and its rows are gone" "$(q 'SELECT count(*) FROM ttl_dn')" "0" + +# Control: the live-NULL case must still be refused, or the fix above is just +# "retire everything" wearing a delete. +psql_run "CREATE TABLE ttl_dl (id int, ts timestamptz, v text) USING pgcolumnar;" +psql_run "INSERT INTO ttl_dl + SELECT g, + CASE WHEN g % 10 = 0 THEN NULL + ELSE now() - interval '400 days' END, + 'v' || g + FROM generate_series(1,900) g;" +psql_run "SELECT pgcolumnar.set_options('ttl_dl', ttl_column => 'ts', + ttl_interval => '90 days');" +# One row deleted, so the table HAS a delete vector and takes the same path as +# ttl_dn above -- but the NULLs are still live. +psql_run "DELETE FROM ttl_dl WHERE id = 1;" +DL_RETIRED="$(q "SELECT pgcolumnar.expire('ttl_dl')")" +check "control: a group whose NULLs are still live is NOT retired" "$DL_RETIRED" "0" +check "control: and those NULL rows survive" \ + "$(q 'SELECT count(*) FROM ttl_dl WHERE ts IS NULL')" "90" + pgc_summary