From 9b552025a98fa1a8d0d75a6ee957809bcdf3fc85 Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Sun, 26 Jul 2026 09:19:01 -0600 Subject: [PATCH 1/3] Fold metadata aggregates per row group, not per storage An ungrouped, unfiltered aggregate over a native table is answered from row group metadata. A zone map describes every row written into its group, deleted ones included, so a group with deletes cannot be folded from its zone map. That was decided once for the whole storage. One deleted row anywhere sent the whole query to a full scan, so a two-million-row table lost a 0.04 ms count(*) to a 95 ms one because a single row was gone, and stayed there until a vacuum. Deletion is a property of a row group, so the decision belongs there. Clean groups fold from their zone maps as before; only the groups that actually have deletes are read, using a new row group restriction on the reader that skips the others in the same claim loop the zone-map predicates already skip in. count(*) needs no data at all even for a dirty group: the live row count is exactly the group's row count minus its deleted count. A count(*)-only query therefore stays on metadata whatever has been deleted. The cost model moves with it, since pricing the path at a full scan the moment any row is deleted hands the planner back the choice this path exists to take away. It also now divides by stripe_row_limit rather than chunk_group_row_limit to count row groups: the latter is the vector size within a group, and using it overstated the group count by the ratio between them, 60 computed against 4 actual at the default limits. That was clamped below the scan cost and so still chose correctly, but it was a wrong model reaching a right answer. Measured on 18.4, 2,000,000 rows in 14 row groups, same machine and session, after deleting one row: before after count(*) 95.07 ms 0.15 ms min/max 118.04 ms 9.91 ms sum(v) 95.50 ms 9.10 ms With nothing deleted the figures are unchanged (0.041 against 0.042 ms for count(*)), because a storage-wide probe still short-circuits the per-group delete lookup when there is nothing to find. --- src/columnar.h | 9 ++ src/columnar_reader.c | 86 ++++++++++- src/columnar_vector.c | 294 +++++++++++++++++++++++++++---------- test/native_agg_deletes.sh | 142 ++++++++++++++++++ 4 files changed, 451 insertions(+), 80 deletions(-) create mode 100755 test/native_agg_deletes.sh diff --git a/src/columnar.h b/src/columnar.h index 471ea61..cfee55e 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -489,6 +489,15 @@ extern void ColumnarEndRead(ColumnarReadState *readState); * stripe indices, so several workers scanning the same relation each claim * distinct stripes. Set by the custom scan's DSM init callbacks. */ +/* + * Restrict a scan to a set of row groups (issue #149). Groups outside the set + * are skipped without their bytes being read. Must be called before the first + * ColumnarReadNextRow; ngroups == 0 makes the scan return no rows. + */ +extern void ColumnarReadRestrictToGroups(ColumnarReadState *readState, + const uint64 *groupNumbers, + int ngroups); + extern void ColumnarReadSetParallelCounter(ColumnarReadState *readState, pg_atomic_uint32 *counter); diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 500c230..f11c723 100644 --- a/src/columnar_reader.c +++ b/src/columnar_reader.c @@ -70,6 +70,17 @@ struct ColumnarReadState SkipPredicate *predicates; /* [numPredicates], in readContext */ int numPredicates; + /* + * Optional restriction to a set of row groups (issue #149). When + * restrictGroups is non-NULL only groups whose groupNumber appears in it are + * read; the rest are passed over without their bytes being touched, exactly as + * a zone-map non-match is. Sorted ascending, in readContext, so the claim loop + * can binary search. The metadata aggregate path uses this to scan only the + * row groups that have deleted rows, folding the others from their zone maps. + */ + uint64 *restrictGroups; /* [numRestrictGroups], sorted, or NULL */ + int numRestrictGroups; + bool started; bool exhausted; @@ -139,6 +150,42 @@ static void columnar_build_predicates(ColumnarReadState *readState, int nkeys, ScanKey keys); static int64 columnar_next_group_index(ColumnarReadState *readState); +/* qsort comparator for the row group restriction set */ +static int +columnar_uint64_cmp(const void *a, const void *b) +{ + uint64 x = *(const uint64 *) a; + uint64 y = *(const uint64 *) b; + + return (x < y) ? -1 : (x > y) ? 1 : 0; +} + +/* + * columnar_group_is_restricted_in + * Is this group number in the read state's restriction set? Binary search + * over the sorted array set by ColumnarReadRestrictToGroups. Only called + * when restrictGroups is non-NULL. + */ +static bool +columnar_group_is_restricted_in(ColumnarReadState *rs, uint64 groupNumber) +{ + int lo = 0; + int hi = rs->numRestrictGroups - 1; + + while (lo <= hi) + { + int mid = lo + (hi - lo) / 2; + + if (rs->restrictGroups[mid] == groupNumber) + return true; + else if (rs->restrictGroups[mid] < groupNumber) + lo = mid + 1; + else + hi = mid - 1; + } + return false; +} + /* ------------------------------------------------------------------------- * value stream codec (shared with the writer) * ------------------------------------------------------------------------- */ @@ -827,7 +874,10 @@ columnar_native_load_group(ColumnarReadState *rs) return false; rg = (NativeRowGroupMetadata *) list_nth(rs->rowGroupList, (int) gi); - if (rs->numPredicates > 0) + if (rs->restrictGroups != NULL && + !columnar_group_is_restricted_in(rs, rg->groupNumber)) + match = false; + else if (rs->numPredicates > 0) { MemoryContext old = MemoryContextSwitchTo(rs->skipContext); @@ -1117,6 +1167,40 @@ ColumnarReadSetParallelCounter(ColumnarReadState *readState, readState->parallelCounter = counter; } +/* + * ColumnarReadRestrictToGroups + * Restrict this scan to the given row group numbers (issue #149). Groups + * outside the set are skipped in the claim loop, so their bytes are never + * read and their column chunks never decoded. The array is copied into the + * read state's own context and sorted there, so the caller may free its own. + * + * Must be called before the first ColumnarReadNextRow. Passing ngroups == 0 + * makes the scan return no rows, which is the honest reading of "restrict to + * nothing" and is what the aggregate path relies on when every group is + * clean. + */ +void +ColumnarReadRestrictToGroups(ColumnarReadState *readState, + const uint64 *groupNumbers, int ngroups) +{ + MemoryContext oldContext; + + Assert(!readState->started); + + oldContext = MemoryContextSwitchTo(readState->readContext); + readState->restrictGroups = (uint64 *) palloc(sizeof(uint64) * + (ngroups > 0 ? ngroups : 1)); + readState->numRestrictGroups = ngroups; + if (ngroups > 0) + { + memcpy(readState->restrictGroups, groupNumbers, + sizeof(uint64) * ngroups); + qsort(readState->restrictGroups, ngroups, sizeof(uint64), + columnar_uint64_cmp); + } + MemoryContextSwitchTo(oldContext); +} + /* ------------------------------------------------------------------------- * Liveness cache (gap 26, phase 4): a projection scan must test each row's base * row number for deletion/visibility. The cache reads the base row-group list diff --git a/src/columnar_vector.c b/src/columnar_vector.c index 4144cdc..43caf01 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -707,75 +707,92 @@ ColumnarCreateUpperPaths(PlannerInfo *root, UpperRelationKind stage, * workers and comes out cheaper, so the planner reads the whole table to * compute what this path takes from row-group metadata (issue #133). * - * With no delete vector the executor answers every aggregate from that - * metadata and reads no data pages, so the work is one metadata entry per row - * group. With deletes it falls back to a real scan that applies the delete - * mask, and the scan cost is the right price. The probe below asks the same - * question execution asks; if the answer changes between planning and - * execution the plan is mispriced, never wrong. + * The executor answers a clean row group from its metadata and reads no data + * pages for it; it reads only the groups that have deletes (issue #149). So + * the price is one metadata entry per row group, plus a scan of the fraction + * of the table that is deleted-in. + * + * Pricing the whole path at the scan cost the moment any row anywhere is + * deleted, as this did, gives the planner back the choice this path exists to + * take away: one deleted row out of six million made a parallel Agg look + * cheaper again and #133 came back. The probe below asks the same question + * execution asks; if the answer changes between planning and execution the + * plan is mispriced, never wrong. */ { - bool hasDeletes = true; + ColumnarOptions opts; + int limit = columnar_stripe_row_limit; + double rows = input_rel->tuples; + double ngroups; + double dirtyFraction = 0.0; Snapshot snap = GetActiveSnapshot(); - + Cost cost; + + /* + * Row groups are governed by stripe_row_limit, not chunk_group_row_limit: + * the latter sets the vector size within a group. Dividing by the vector + * size overstated the group count by the ratio between them -- at the + * default limits, 60 computed groups against 4 actual. The estimate was + * clamped below the scan cost so it still chose right, but it was a wrong + * model getting a right answer, and it stops being harmless as soon as the + * clamp is not what decides. + */ + if (ColumnarReadOptions(relid, &opts) && + opts.stripeRowLimitSet && opts.stripeRowLimit > 0) + limit = opts.stripeRowLimit; + if (limit <= 0) + limit = 1; + + /* + * A never-analyzed relation has no row estimate. Deriving one from the + * page count keeps the cost tied to something real, so a missing estimate + * cannot make this path look free. + */ + if (rows < 0) + rows = (input_rel->pages > 0) + ? (double) input_rel->pages * 100.0 + : 1000.0; + ngroups = ceil(rows / (double) limit); + if (ngroups < 1) + ngroups = 1; + + /* + * What fraction of the groups must actually be read. Counting them exactly + * would mean a delete-vector lookup per group at planning time, which is + * the per-group catalog traffic this path exists to avoid paying. The + * storage-wide probe is one lookup and distinguishes the case that matters + * -- nothing deleted at all -- from the case where something is. When + * something is, assume one group in four is affected rather than all of + * them: wrong in both directions by a bounded factor, where the previous + * all-or-nothing was wrong by the whole table. + */ if (snap != NULL) { Relation frel = table_open(relid, AccessShareLock); - hasDeletes = - ColumnarStorageHasDeleteVector(ColumnarStorageId(frel), - ColumnarCatalogSnapshot(snap)); + if (ColumnarStorageHasDeleteVector(ColumnarStorageId(frel), + ColumnarCatalogSnapshot(snap))) + dirtyFraction = 0.25; table_close(frel, AccessShareLock); } - - if (hasDeletes) - { - cpath->path.startup_cost = cheapest->total_cost; - cpath->path.total_cost = cheapest->total_cost; - } else - { - ColumnarOptions opts; - int limit = columnar_chunk_group_row_limit; - double rows = input_rel->tuples; - double ngroups; - Cost cost; - - /* the table's own limit when it sets one, else the GUC default */ - if (ColumnarReadOptions(relid, &opts) && - opts.chunkGroupRowLimitSet && opts.chunkGroupRowLimit > 0) - limit = opts.chunkGroupRowLimit; - if (limit <= 0) - limit = 1; - - /* - * A never-analyzed relation has no row estimate. Deriving one from - * the page count keeps the cost tied to something real, so a missing - * estimate cannot make this path look free. - */ - if (rows < 0) - rows = (input_rel->pages > 0) - ? (double) input_rel->pages * 100.0 - : 1000.0; - ngroups = ceil(rows / (double) limit); - if (ngroups < 1) - ngroups = 1; - - cost = cpu_tuple_cost * 10; /* getting to the catalog */ - cost += ngroups * (cpu_tuple_cost + - cpu_operator_cost * (double) naggs); - - /* reading metadata is never dearer than reading the table */ - if (cost > cheapest->total_cost) - cost = cheapest->total_cost; - - /* - * One row comes out, and only after every row group is folded in, so - * there is no partial result to start up cheaply with. - */ - cpath->path.startup_cost = cost; - cpath->path.total_cost = cost; - } + dirtyFraction = 0.25; + + cost = cpu_tuple_cost * 10; /* getting to the catalog */ + cost += ngroups * (cpu_tuple_cost + + cpu_operator_cost * (double) naggs); + cost += dirtyFraction * cheapest->total_cost; + + /* reading metadata is never dearer than reading the table */ + if (cost > cheapest->total_cost) + cost = cheapest->total_cost; + + /* + * One row comes out, and only after every row group is folded in, so + * there is no partial result to start up cheaply with. + */ + cpath->path.startup_cost = cost; + cpath->path.total_cost = cost; } cpath->path.pathkeys = NIL; cpath->flags = 0; @@ -1023,17 +1040,85 @@ columnar_agg_finalize(ColumnarAggSpec *spec, bool *isnull) return (Datum) 0; } +/* + * columnar_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 +columnar_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 = ColumnarReadDeleteVectorList(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; +} + /* * columnar_fill_native_metadata_agg * Answer an ungrouped, unfiltered aggregate over a native (PGCN v1) table - * entirely from its whole-chunk zone maps (native spec 7.1, D5b): count(*) - * from row-group row counts, count(col) and the avg count from value_count, - * sum and the avg sum from the zone int sum (int2/int4), and min/max from the + * from its whole-chunk zone maps (native spec 7.1, D5b): count(*) from + * row-group row counts, count(col) and the avg count from value_count, sum + * and the avg sum from the zone int sum (int2/int4), and min/max from the * zone min/max. The upper-path hook adds this path only when every aggregate - * is so answerable and there is no filter, so no data pages are read. + * is so answerable and there is no filter. + * + * Deletes are handled per row group rather than per storage (issue #149). A + * zone map describes every row written into its group, deleted ones + * included, so a group with deletes cannot be folded from its zone map. It + * used to be that one deleted row anywhere disabled this path for the whole + * table, and a six-million-row table lost a 0.02 ms count(*) to a 222 ms scan + * because a single row was gone. Deletion is a property of a group, so the + * decision belongs there: clean groups fold from their zone maps, and only + * the groups that actually have deletes are read. + * + * Returns the groups that must be scanned, by group number, with *ndirty set + * to their count. A count(*)-only query never returns any: count(*) over a + * group is rowCount minus the deleted count, which is exact and needs no + * data pages even when the group has deletes. */ -static void -columnar_fill_native_metadata_agg(ColumnarAggScanState *state) +static uint64 * +columnar_fill_native_metadata_agg(ColumnarAggScanState *state, int *ndirty) { EState *estate = state->css.ss.ps.state; Relation rel; @@ -1043,6 +1128,8 @@ columnar_fill_native_metadata_agg(ColumnarAggScanState *state) List *groups; ListCell *lc; bool needZones = false; + bool anyDeletes; + uint64 *dirty; int na; /* @@ -1068,13 +1155,52 @@ columnar_fill_native_metadata_agg(ColumnarAggScanState *state) snap = ColumnarCatalogSnapshot(estate->es_snapshot); storageId = ColumnarStorageId(rel); + /* + * One storage-wide probe first. When nothing is deleted no group can have + * deletes, so the per-group delete lookup below is pure cost; skipping it + * keeps a clean table at exactly the catalog traffic it had before this + * change, which for a count(*) is the row group list and nothing else. + */ + anyDeletes = ColumnarStorageHasDeleteVector(storageId, snap); + groups = ColumnarReadRowGroupList(storageId, snap); + dirty = palloc(sizeof(uint64) * (list_length(groups) > 0 + ? list_length(groups) : 1)); + *ndirty = 0; + foreach(lc, groups) { NativeRowGroupMetadata *rg = (NativeRowGroupMetadata *) lfirst(lc); NativeZoneMapMetadata **byCol = NULL; + uint64 deleted = 0; int a; + if (anyDeletes) + deleted = columnar_group_deleted_count(storageId, rg, snap); + + if (deleted > 0) + { + /* + * This group's zone maps describe deleted rows too, so they cannot + * answer anything that reads a value. count(*) is the exception: the + * live row count is exactly rowCount - deleted, so a count(*)-only + * query stays on metadata even here. Anything else defers the whole + * group to the scan, which folds every aggregate for it -- including + * count(*), so nothing is counted twice. + */ + if (!needZones) + { + for (a = 0; a < state->naggs; a++) + { + Assert(state->specs[a].kind == COLUMNAR_AGG_COUNT_STAR); + state->specs[a].count += (int64) (rg->rowCount - deleted); + } + } + else + dirty[(*ndirty)++] = rg->groupNumber; + continue; + } + if (needZones) { List *zones = ColumnarReadZoneMapList(storageId, @@ -1161,6 +1287,7 @@ columnar_fill_native_metadata_agg(ColumnarAggScanState *state) } table_close(rel, AccessShareLock); + return dirty; } /* @@ -1170,9 +1297,14 @@ columnar_fill_native_metadata_agg(ColumnarAggScanState *state) * case where the zone-map-only path cannot be used because the storage has * deletes (D6b). No quals: the upper-path hook only adds the native agg path * when there is no filter. + * + * When restrictGroups is non-NULL the scan is confined to those row groups + * (issue #149), so only the groups that have deletes are read; the rest were + * already folded from their zone maps by the caller. */ static void -columnar_native_scan_agg(ColumnarAggScanState *state) +columnar_native_scan_agg(ColumnarAggScanState *state, + const uint64 *restrictGroups, int nRestrictGroups) { EState *estate = state->css.ss.ps.state; Relation rel = table_open(state->relid, AccessShareLock); @@ -1194,6 +1326,8 @@ columnar_native_scan_agg(ColumnarAggScanState *state) ColumnarFlushDeleteVectorForRelation(rel); rs = ColumnarBeginRead(rel, estate->es_snapshot, NULL, projected, 0, NULL); + if (restrictGroups != NULL) + ColumnarReadRestrictToGroups(rs, restrictGroups, nRestrictGroups); while (ColumnarReadNextRow(rs, values, nulls, &rowNumber)) { for (a = 0; a < state->naggs; a++) @@ -1219,32 +1353,34 @@ ColumnarExecAggScan(CustomScanState *node) ExprContext *econtext = node->ss.ps.ps_ExprContext; TupleTableSlot *result; int a; - EState *estate = node->ss.ps.state; Relation frel; - bool hasDeletes; + uint64 *dirtyGroups; + int nDirtyGroups; if (state->done) return NULL; state->done = true; /* - * A native table answers from zone maps when no rows are deleted (native spec - * 7.1): the upper-path hook added this path only when every aggregate is - * zone-map answerable and there is no filter, so no data pages are read. When - * the storage has deletes the zone maps include deleted rows, so fall back to a - * scan that applies the delete mask. + * A native table answers from zone maps (native spec 7.1): the upper-path + * hook added this path only when every aggregate is zone-map answerable and + * there is no filter. A zone map covers deleted rows too, so a row group with + * deletes cannot be folded from it; those groups are scanned instead, and only + * those (issue #149). + * + * The delete vector must be flushed before any of this. A delete made earlier + * in this transaction can still be sitting in the per-relation buffer, and a + * group whose deletes are unflushed reads as clean -- which would fold it from + * a zone map that counts the rows this transaction has already removed. */ frel = table_open(state->relid, AccessShareLock); ColumnarFlushWriteStateForRelation(state->relid); ColumnarFlushDeleteVectorForRelation(frel); - hasDeletes = ColumnarStorageHasDeleteVector(ColumnarStorageId(frel), - ColumnarCatalogSnapshot(estate->es_snapshot)); table_close(frel, AccessShareLock); - if (hasDeletes) - columnar_native_scan_agg(state); - else - columnar_fill_native_metadata_agg(state); + dirtyGroups = columnar_fill_native_metadata_agg(state, &nDirtyGroups); + if (nDirtyGroups > 0) + columnar_native_scan_agg(state, dirtyGroups, nDirtyGroups); state->haveStats = false; /* build the single result row from the finalized aggregates */ diff --git a/test/native_agg_deletes.sh b/test/native_agg_deletes.sh new file mode 100755 index 0000000..a2664aa --- /dev/null +++ b/test/native_agg_deletes.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# +# pgColumnar metadata aggregates against a table with deletes (issue #149). +# +# An ungrouped, unfiltered aggregate over a native table is answered from row +# group metadata. A zone map describes every row written into its group, deleted +# ones included, so a group with deletes cannot be folded from its zone map. That +# used to be decided for the whole storage: one deleted row anywhere sent the +# query to a full scan, and a table lost a 0.04 ms count(*) to a 95 ms scan +# because one row of two million was gone. +# +# Deletion is a property of a row group, so the decision belongs there. Three +# things are asserted, needing three kinds of evidence. +# +# 1. The answers are right. Differential against a heap mirror over four delete +# patterns, ending with one that dirties every group, because the interesting +# failure is a group folded twice or not at all -- once from its zone map and +# once from the scan that covers it. +# +# 2. count(*) no longer cares. Timed as a ratio against the same query on the +# same table with nothing deleted, because a millisecond threshold is not +# portable. count(*) over a group is its row count minus its deleted count, +# which is exact, so a delete should cost it almost nothing. Before the change +# this ratio was in the thousands. +# +# 3. Only the dirty groups are read. min/max cannot be folded from a zone map +# once a row is gone, so those groups are scanned -- but only those. Timed +# against the same query with vectorization off, which reads everything: one +# dirty group out of many must come in far under a full scan. Before the +# change the two were the same query. +# +# Usage: test/native_agg_deletes.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}" + +ROWS=${PGC_AGGDEL_ROWS:-400000} + +psql_run "DROP TABLE IF EXISTS ad_c; DROP TABLE IF EXISTS ad_h; + CREATE TABLE ad_c (id int, v int, w bigint, s text) USING pgcolumnar; + INSERT INTO ad_c SELECT g, g % 1000, g * 7, 'r' || g + FROM generate_series(1, $ROWS) g; + CREATE TABLE ad_h (id int, v int, w bigint, s text); + INSERT INTO ad_h SELECT g, g % 1000, g * 7, 'r' || g + FROM generate_series(1, $ROWS) g;" >/dev/null 2>&1 + +groups="$(q "SELECT count(*) FROM pgcolumnar.row_group r + JOIN pgcolumnar.storage s ON s.storage_id = r.storage_id + WHERE s.relation_id = 'ad_c'::regclass;")" +echo "-- $ROWS rows in ${groups} row groups" + +# --- 1. the answers are right -------------------------------------------------- + +AGGS="count(*) count(v) count(s) sum(v) avg(v) min(v) max(v) min(w) max(w) min(s) max(s)" + +differential() { # label + local bad="" agg cv hv + + for agg in $AGGS; do + cv="$(q "SELECT $agg FROM ad_c;")" + hv="$(q "SELECT $agg FROM ad_h;")" + [ "$cv" = "$hv" ] || bad="$bad $agg(columnar=$cv heap=$hv)" + done + check "aggregates match heap $1" "${bad:-same}" "same" +} + +differential "with nothing deleted" + +psql_run "DELETE FROM ad_c WHERE id = $((ROWS / 2)); + DELETE FROM ad_h WHERE id = $((ROWS / 2));" >/dev/null 2>&1 +differential "after one row is deleted" + +psql_run "DELETE FROM ad_c WHERE id BETWEEN 1 AND $((ROWS / 10)); + DELETE FROM ad_h WHERE id BETWEEN 1 AND $((ROWS / 10));" >/dev/null 2>&1 +differential "after a contiguous tenth is deleted" + +# every group now has deletes, so every group takes the scan path +psql_run "DELETE FROM ad_c WHERE id % 7 = 0; + DELETE FROM ad_h WHERE id % 7 = 0;" >/dev/null 2>&1 +differential "after every group is dirtied" + +# --- 2. count(*) no longer cares about a delete -------------------------------- + +# a fresh pair: the table above is now heavily deleted, and this measures the +# cost of *a* delete, not of many +psql_run "DROP TABLE IF EXISTS ad_t; + CREATE TABLE ad_t (id int, v int) USING pgcolumnar; + INSERT INTO ad_t SELECT g, g % 1000 FROM generate_series(1, $ROWS) g;" >/dev/null 2>&1 + +# server-side timing, best of three: psql's connect floor is ~15 ms and would +# swamp a sub-millisecond query +ms() { # sql -> milliseconds, best of 3 + local out best="" t + out="$(psql_run "\\timing on + $1 + $1 + $1" 2>/dev/null | grep -E '^Time:' | awk '{print $2}')" + for t in $out; do + if [ -z "$best" ] || awk -v a="$t" -v b="$best" 'BEGIN{exit !(a/dev/null 2>&1 +dirty_ms="$(ms "SELECT count(*) FROM ad_t;")" + +echo "-- count(*): ${clean_ms} ms with no deletes, ${dirty_ms} ms with one" + +check "one deleted row does not put count(*) into a different class" \ + "$(awk -v c="$clean_ms" -v d="$dirty_ms" \ + 'BEGIN { print (c > 0 && d / c < 20) ? "yes" : "no (" d "/" c ")" }')" \ + "yes" + +# --- 3. only the groups with deletes are read ---------------------------------- + +# min/max cannot come from a zone map once a row in that group is gone, so the +# group is scanned. One dirty group out of many must still cost far less than +# reading the table, which is what the vectorization-off path does. +mm_ms="$(ms "SELECT min(v), max(v) FROM ad_t;")" +full_ms="$(ms "SET pgcolumnar.enable_vectorization = off; + SELECT min(v), max(v) FROM ad_t;")" + +echo "-- min/max: ${mm_ms} ms with one group dirty, ${full_ms} ms reading everything" + +check "a dirty group is scanned without scanning the clean ones" \ + "$(awk -v m="$mm_ms" -v f="$full_ms" \ + 'BEGIN { print (f > 0 && m < f / 2) ? "yes" : "no (" m " vs " f ")" }')" \ + "yes" + +# and the path is still the one being measured +plan="$(q "EXPLAIN (COSTS off) SELECT count(*) FROM ad_t;" | head -1)" +check "the metadata aggregate path is still chosen with a row deleted" \ + "$(case "$plan" in *ColumnarScan*) echo yes ;; *) echo "no ($plan)" ;; esac)" \ + "yes" + +pgc_summary From bbc3f30eaaf0591bfc712ce7428b894ba6eececb Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Sun, 26 Jul 2026 09:30:55 -0600 Subject: [PATCH 2/3] Add native_agg_deletes: the delete cliff, timed as a ratio Correctness is differential against a heap mirror over four delete patterns, ending with one that dirties every row group, because the failure this change could introduce is a group folded twice or not at all -- once from its zone map and once by the scan that covers it. The two timing checks are what discriminate. On f7adbdb they fail: count(*): 0.0210 ms with no deletes, 15.8100 ms with one (753x) min/max: 21.2350 ms with one group dirty, 30.6990 reading everything and on this branch they pass: count(*): 0.0210 ms with no deletes, 0.1340 ms with one min/max: 8.4050 ms with one group dirty, 30.2770 reading everything Both are ratios against a measurement the same run takes, not millisecond thresholds, so they carry across machines. --- test/native_agg_deletes.sh | 46 +++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/test/native_agg_deletes.sh b/test/native_agg_deletes.sh index a2664aa..47bd056 100755 --- a/test/native_agg_deletes.sh +++ b/test/native_agg_deletes.sh @@ -49,7 +49,7 @@ psql_run "DROP TABLE IF EXISTS ad_c; DROP TABLE IF EXISTS ad_h; groups="$(q "SELECT count(*) FROM pgcolumnar.row_group r JOIN pgcolumnar.storage s ON s.storage_id = r.storage_id - WHERE s.relation_id = 'ad_c'::regclass;")" + WHERE s.relation_oid = 'ad_c'::regclass;")" echo "-- $ROWS rows in ${groups} row groups" # --- 1. the answers are right -------------------------------------------------- @@ -90,25 +90,31 @@ psql_run "DROP TABLE IF EXISTS ad_t; CREATE TABLE ad_t (id int, v int) USING pgcolumnar; INSERT INTO ad_t SELECT g, g % 1000 FROM generate_series(1, $ROWS) g;" >/dev/null 2>&1 -# server-side timing, best of three: psql's connect floor is ~15 ms and would -# swamp a sub-millisecond query -ms() { # sql -> milliseconds, best of 3 - local out best="" t - out="$(psql_run "\\timing on - $1 - $1 - $1" 2>/dev/null | grep -E '^Time:' | awk '{print $2}')" - for t in $out; do - if [ -z "$best" ] || awk -v a="$t" -v b="$best" 'BEGIN{exit !(a/dev/null 2>&1 + +# A leading SET prints its own "SET" line ahead of the result, so take the last +# line rather than the first. +ms() { # sql [session settings] -> milliseconds, best of 3 + q "${2:-} SELECT ad_ms(\$q\$$1\$q\$, 3);" | tail -1 } -clean_ms="$(ms "SELECT count(*) FROM ad_t;")" +clean_ms="$(ms "SELECT count(*) FROM ad_t")" psql_run "DELETE FROM ad_t WHERE id = $((ROWS / 2));" >/dev/null 2>&1 -dirty_ms="$(ms "SELECT count(*) FROM ad_t;")" +dirty_ms="$(ms "SELECT count(*) FROM ad_t")" echo "-- count(*): ${clean_ms} ms with no deletes, ${dirty_ms} ms with one" @@ -122,9 +128,9 @@ check "one deleted row does not put count(*) into a different class" \ # min/max cannot come from a zone map once a row in that group is gone, so the # group is scanned. One dirty group out of many must still cost far less than # reading the table, which is what the vectorization-off path does. -mm_ms="$(ms "SELECT min(v), max(v) FROM ad_t;")" -full_ms="$(ms "SET pgcolumnar.enable_vectorization = off; - SELECT min(v), max(v) FROM ad_t;")" +mm_ms="$(ms "SELECT min(v), max(v) FROM ad_t")" +full_ms="$(ms "SELECT min(v), max(v) FROM ad_t" \ + "SET pgcolumnar.enable_vectorization = off;")" echo "-- min/max: ${mm_ms} ms with one group dirty, ${full_ms} ms reading everything" From 2c9912cee99bce3efc0546b14a29b105e8bb6e3c Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Sun, 26 Jul 2026 10:26:46 -0600 Subject: [PATCH 3/3] Register native_agg_deletes in the matrix and the testing docs The suite was added but not listed in test/run_all_versions.sh, so the gate never ran it: a green PG18 and PG19 matrix on this branch reported 71 suites and none of them was this one. A test that no gate runs stops being a test the first time someone changes the code under it. Also list it in docs/testing.md beside the suite it extends. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb --- docs/testing.md | 1 + test/run_all_versions.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/testing.md b/docs/testing.md index ad7d846..792687b 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -53,6 +53,7 @@ test/native_encoding.sh /path/to/pg_config # native per-vector encoding cascad test/native_zonemap.sh /path/to/pg_config # native zone maps test/native_skip.sh /path/to/pg_config # native chunk and vector skipping test/native_agg.sh /path/to/pg_config # native aggregate paths +test/native_agg_deletes.sh /path/to/pg_config # per-row-group fold when rows are deleted test/native_bloom.sh /path/to/pg_config # native per-chunk bloom filters test/native_vecskip.sh /path/to/pg_config # native per-vector skipping test/native_index.sh /path/to/pg_config # native index and index scan diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 8cfe1b5..b6cb099 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -27,7 +27,7 @@ SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" SUITES=(harness_selftest smoke phase2 phase3 phase4 phase5 phase6 audit concurrency unique_conc \ differential recovery fuzz hardening concurrent_diff parallel sorted_projection \ arrow_export parquet_export read_stream corruption \ - generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_zonemap native_skip native_agg native_bloom native_vecskip native_index native_dml native_ios native_projection native_cluster native_compact native_recluster native_reclaim native_ownership native_reclaim_cycles native_reclaim_frag native_gap native_truncate native_rewrite native_rewrite_conc native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel wal_envelope decode_interrupts native_fetch_cache isolation) + generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_zonemap native_skip native_agg native_agg_deletes native_bloom native_vecskip native_index native_dml native_ios native_projection native_cluster native_compact native_recluster native_reclaim native_ownership native_reclaim_cycles native_reclaim_frag native_gap native_truncate native_rewrite native_rewrite_conc native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel wal_envelope decode_interrupts native_fetch_cache isolation) # Default matrix: one assert-enabled pg_config per major, 15 through 19. DEFAULT_CONFIGS=(