From 3ecde7f997eee209966968336701cc207d08487a Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 01:14:59 +0000 Subject: [PATCH 1/3] test: the grouped node's float sums raise on overflow, as core's do (#1341 item 50) Two finite float8s, or two finite reals, whose sum is infinite raise "value out of range: overflow" (22003) in core's float8pl and float4pl. The grouped node's accumulators carry the same check, and nothing tested it: only avg's overflow had an arm, in the shell alone. native_groupagg gains the sum arm in both twins, judged by SQLSTATE against core with the node off, plus a check that the node fires. Removing either accumulator's check reddens its own position. native_groupagg enters the ledger seeded; suites_not_covered 234 -> 233. cluster_tests +1. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/check_ledger.tsv | 94 +++++++++++++++++++++++++++++ test/check_ledger_budget.txt | 15 ++++- test/native_groupagg.sh | 21 +++++++ test/pytest/TESTS.md | 1 + test/pytest/expected_tests.txt | 4 +- test/pytest/test_native_groupagg.py | 26 ++++++++ 6 files changed, 158 insertions(+), 3 deletions(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index bace8d67b..f5b5c01b4 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -2059,6 +2059,100 @@ native_fetch_coalesce native_fetch_coalesce premise: fetching one projected colu native_fetch_coalesce native_fetch_coalesce premise: the coalescing helper holds both the bound and the validity copy 15;16;17;18;19 never - native_fetch_coalesce native_fetch_coalesce the validity copy is bounded by the chunk length before it runs 15;16;17;18;19 never - native_fetch_coalesce native_fetch_coalesce the wide fetch returns the projected values 15;16;17;18;19 2026-10-14 every column of a coalesced fetch is copied from the start of the span (off = 0) +native_groupagg native_groupagg a NULL key counts as a group in the bucket bound: four groups under a cap of 3 are declined and all answered 15;16;17;18;19 never - +native_groupagg native_groupagg a cached plan run under a lower cap stops at the runtime cap, with its hint 15;16;17;18;19 never - +native_groupagg native_groupagg a date_bin key is bounded as a truncating key is 15;16;17;18;19 never - +native_groupagg native_groupagg a five-minute bucket query over more rows than the cap still takes the grouped path 15;16;17;18;19 never - +native_groupagg native_groupagg a lookalike function with the same signature gets NO bound 15;16;17;18;19 never - +native_groupagg native_groupagg a mixed timestamp/timestamptz predicate never narrows the estimate below the true count 15;16;17;18;19 never - +native_groupagg native_groupagg a non-time expression key gets no bound either 15;16;17;18;19 never - +native_groupagg native_groupagg a plain key over more rows than the cap is declined 15;16;17;18;19 never - +native_groupagg native_groupagg a skewed key whose estimate is far too low is declined under a cap of 30,000, and answers every group 15;16;17;18;19 never - +native_groupagg native_groupagg an informed estimate (plain column) is not replaced 15;16;17;18;19 never - +native_groupagg native_groupagg and a date_bin key with no time bound is bounded from the zone maps 15;16;17;18;19 never - +native_groupagg native_groupagg and it is an UPPER bound: the true group count is at or below it 15;16;17;18;19 never - +native_groupagg native_groupagg and the declined query answers what heap answers 15;16;17;18;19 never - +native_groupagg native_groupagg answer: non-deterministic collation grouping is case-insensitive (10 groups) 15;16;17;18;19 never - +native_groupagg native_groupagg avg(float8) overflow errors like core 15;16;17;18;19 never - +native_groupagg native_groupagg avg(float8) overflow: node fires 15;16;17;18;19 never - +native_groupagg native_groupagg by default a five-minute bucket query takes the grouped path 15;16;17;18;19 never - +native_groupagg native_groupagg empty table: grouped scan yields 0 rows 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY added column 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY added column [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY host 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY host after deletes 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY host after deletes [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY host, added column 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY host, added column [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY hour 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY hour [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY hour, host (q4/q5 shape) 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY hour, host (q4/q5 shape) [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY integer expr key 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY integer expr key [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY region, host (two text keys) 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY region, host (two text keys) [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY smallint column key 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: GROUP BY smallint column key [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: NULL ts group folds like heap 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: NULL ts group folds like heap [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: WHERE on measure + GROUP BY host 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: WHERE on measure + GROUP BY host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: WHERE range + GROUP BY hour, host 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: WHERE range + GROUP BY hour, host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: all rows filtered out -> 0 groups 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: default cap runs the high-cardinality key 15;16;17;18;19 never - +native_groupagg native_groupagg oracle exact: f(key) output still correct 15;16;17;18;19 never - +native_groupagg native_groupagg plan: GROUP BY hour expr uses grouped node 15;16;17;18;19 never - +native_groupagg native_groupagg plan: GROUP BY text key uses grouped node 15;16;17;18;19 never - +native_groupagg native_groupagg plan: GROUP BY with no aggregate falls back 15;16;17;18;19 never - +native_groupagg native_groupagg plan: f(key) output falls back 15;16;17;18;19 never - +native_groupagg native_groupagg plan: non-deterministic collation key falls back 15;16;17;18;19 never - +native_groupagg native_groupagg plan: off -> ordinary Agg, not grouped node 15;16;17;18;19 never - +native_groupagg native_groupagg plan: q4-shape (WHERE+2 keys) uses grouped node 15;16;17;18;19 never - +native_groupagg native_groupagg plan: the node is chosen for a key well under the gate 15;16;17;18;19 never - +native_groupagg native_groupagg planning a bucketed GROUP BY scans zone_map once, not once per row group 15;16;17;18;19 never - +native_groupagg native_groupagg premise: ANALYZE estimates the skewed key's 40,001 groups below 3,000 15;16;17;18;19 never - +native_groupagg native_groupagg premise: and so is ten, so the two costs are comparable 15;16;17;18;19 never - +native_groupagg native_groupagg premise: one aggregate is costed as the grouped node 15;16;17;18;19 never - +native_groupagg native_groupagg premise: the cost fixture holds the rows the 450.00 is computed from 15;16;17;18;19 never - +native_groupagg native_groupagg premise: the fixture has many row groups, so one scan and one per group differ 15;16;17;18;19 never - +native_groupagg native_groupagg premise: the grouped path is costed at all (non-empty estimate) 15;16;17;18;19 never - +native_groupagg native_groupagg premise: the mixed-type window holds data 15;16;17;18;19 never - +native_groupagg native_groupagg raising groupagg_max_groups to 10,000,000 lets the same query take the grouped path 15;16;17;18;19 never - +native_groupagg native_groupagg regress B1: sum(real) fires the node 15;16;17;18;19 never - +native_groupagg native_groupagg regress B1: sum(real) is nonzero 15;16;17;18;19 never - +native_groupagg native_groupagg regress B1: sum(real) matches heap, not 0 15;16;17;18;19 never - +native_groupagg native_groupagg regress B2: gating WHERE (SELECT false) returns no rows 15;16;17;18;19 never - +native_groupagg native_groupagg regress B2: gating WHERE (SELECT true) is a no-op 15;16;17;18;19 never - +native_groupagg native_groupagg regress B2: gating WHERE falls back (not the node) 15;16;17;18;19 never - +native_groupagg native_groupagg sum(float8) and sum(real) overflow raise 22003, as core does 15;16;17;18;19 2026-10-15 the node's float8 sum without its overflow check +native_groupagg native_groupagg sum(float8) and sum(real) overflow: node fires 15;16;17;18;19 never - +native_groupagg native_groupagg ten aggregates cost more than one (#349) 15;16;17;18;19 never - +native_groupagg native_groupagg the bound is accurate for a truncating time key, not just smaller 15;16;17;18;19 never - +native_groupagg native_groupagg the grouped path is on by default 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY host 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY host after deletes 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY host after deletes [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY hour, host 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY hour, host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY region, host, WHERE 15;16;17;18;19 never - +native_groupagg native_groupagg toggle exact: GROUP BY region, host, WHERE [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle float/avg: GROUP BY host 15;16;17;18;19 never - +native_groupagg native_groupagg toggle float/avg: GROUP BY host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle float/avg: GROUP BY hour, host 15;16;17;18;19 never - +native_groupagg native_groupagg toggle float/avg: GROUP BY hour, host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle float/avg: WHERE + GROUP BY region, host 15;16;17;18;19 never - +native_groupagg native_groupagg toggle float/avg: WHERE + GROUP BY region, host [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle min/max numeric display-scale tie 15;16;17;18;19 never - +native_groupagg native_groupagg toggle min/max numeric display-scale tie [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg toggle sum signed-zero (-0 preserved) 15;16;17;18;19 never - +native_groupagg native_groupagg toggle sum signed-zero (-0 preserved) [node fires] 15;16;17;18;19 never - +native_groupagg native_groupagg while a matching-type predicate still gets one 15;16;17;18;19 never - +native_groupagg native_groupagg with no time bound at all, a truncating key is bounded from the zone maps 15;16;17;18;19 never - +native_groupagg native_groupagg with only a lower time bound, a truncating key is still bounded 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off 15;16;17;18;19 2026-09-23 the runtime bloom probe's verdict inverted, so the filter rejects what matches native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table plan has coordinator 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 611c7ee8d..3a800607d 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 232 +suites_not_covered 231 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -2296,4 +2296,15 @@ suites_not_covered 232 # never 2095 + 84 = 2179 # dated 691 + 3 = 694 # 2179 + 694 == 2873 -checks_never_observed_red 2179 +# +# #1341 item 50, the grouped node's float sums raise on overflow (jdatcmd): 2179 -> +# 2272, and suites_not_covered 232 -> 231. native_groupagg had no rows; it enters +# with all 94 checks from five-major runs (94/94 on each of 15-19). The new sum arm +# is dated by the node's float8 sum without its overflow check; the other 93 arrive +# never: seeding a suite moves the census up by its undated rows. +# +# ledger rows 2873 + 94 = 2967 +# never 2179 + 93 = 2272 +# dated 694 + 1 = 695 +# 2272 + 695 == 2967 +checks_never_observed_red 2272 diff --git a/test/native_groupagg.sh b/test/native_groupagg.sh index e2a50355e..efa01cbc1 100755 --- a/test/native_groupagg.sh +++ b/test/native_groupagg.sh @@ -319,6 +319,27 @@ ov_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postg check "avg(float8) overflow errors like core" \ "$(grep -qi 'out of range' <<<"$ov_out" && echo yes || echo no)" yes +# sum overflow parity (#1341 item 50): two finite float8s, or two finite reals, whose +# sum is infinite raise "value out of range: overflow" (22003) in core's float8pl and +# float4pl. The node's accumulators must raise it too rather than return Infinity. +# Judged by SQLSTATE, against core with the node turned off. +psql_run "DROP TABLE IF EXISTS ovs_col; + CREATE TABLE ovs_col (k int, d float8, r real) USING pgcolumnar; + INSERT INTO ovs_col VALUES (1, 1e308, 3e38), (1, 1e308, 3e38);" >/dev/null +gv_state() { # gv_state on|off QUERY -> ok, or the SQLSTATE the query raised + local out + out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \ + -X -At -v ON_ERROR_STOP=1 -v VERBOSITY=sqlstate \ + -c "SET pgcolumnar.enable_group_vectorization = $1;" -c "$2" 2>&1 >/dev/null)" + [ -z "$out" ] && { echo ok; return; } + sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' <<<"$out" | head -1 +} +check "sum(float8) and sum(real) overflow: node fires" \ + "$(pgc_is_groupvec "SELECT k, sum(d) FROM ovs_col GROUP BY k")|$(pgc_is_groupvec "SELECT k, sum(r) FROM ovs_col GROUP BY k")" "yes|yes" +check "sum(float8) and sum(real) overflow raise 22003, as core does" \ + "$(gv_state on "SELECT k, sum(d) FROM ovs_col GROUP BY k")|$(gv_state on "SELECT k, sum(r) FROM ovs_col GROUP BY k")|$(gv_state off "SELECT k, sum(d) FROM ovs_col GROUP BY k")|$(gv_state off "SELECT k, sum(r) FROM ovs_col GROUP BY k")" \ + "22003|22003|22003|22003" + # ---- 8. degenerate inputs -------------------------------------------------- diff_query "oracle exact: all rows filtered out -> 0 groups" \ diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 02a95e89c..0e597cec2 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -5706,6 +5706,7 @@ every major for this pair, which is the designed third state and not a failure. | `test_an_output_built_on_a_key_falls_back_and_still_answers` | `f(key)` output and a bare `GROUP BY` with no aggregate both fall back, and the answer is still right | | `test_a_non_deterministic_collation_key_falls_back` | the fallback happens and the grouping is case-insensitive; declines under the shell suite's own name where ICU is absent | | `test_sum_real_matches_heap_rather_than_returning_zero` | blocker 1: the node fires, matches heap, and is not zero | +| `test_a_float_sum_that_overflows_raises_as_core_does` | two finite float8s, or reals, whose sum is infinite raise `22003` through the node, as core's `float8pl` and `float4pl` do with it off (#1341 item 50) | | `test_a_gating_where_is_honoured_rather_than_dropped` | blocker 2: a one-time filter makes the node decline, and both the false and true gates answer as heap does | | `test_avg_float8_overflows_the_way_core_overflows` | the node fires and raises out of range, which a row comparison cannot express | | `test_degenerate_inputs_produce_no_groups` | a predicate matching nothing, and an empty table | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index db6bdedfb..a7c06924a 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -673,4 +673,6 @@ guard_tests 435 # in test_am_contracts.py, by collection over the cluster files on this tree. # cluster_tests 566 -> 567 for #1420's wiring arm in test_connection.py, by collection # over the cluster files on this tree. -cluster_tests 567 +# cluster_tests 567 -> 568 for #1341's test_a_float_sum_that_overflows_raises_as_core_does in +# test_native_groupagg.py, by collection over the cluster files on this tree. +cluster_tests 568 diff --git a/test/pytest/test_native_groupagg.py b/test/pytest/test_native_groupagg.py index cb4764693..c7219c5e2 100644 --- a/test/pytest/test_native_groupagg.py +++ b/test/pytest/test_native_groupagg.py @@ -434,6 +434,32 @@ def test_sum_real_matches_heap_rather_than_returning_zero(pgc_conn, expect): expect.rows(zeroes, [("True",)], "regress B1: sum(real) is nonzero") +def test_a_float_sum_that_overflows_raises_as_core_does(pgc_conn, expect): + """Two finite float8s, or two finite reals, whose sum is infinite raise 22003 in + core's float8pl and float4pl (#1341 item 50). The node's accumulators must raise + it too rather than return Infinity. Judged by SQLSTATE, against core with the + node turned off.""" + _exec(pgc_conn, "CREATE TABLE ovs_col (k int, d float8, r real) USING pgcolumnar") + _exec(pgc_conn, "INSERT INTO ovs_col VALUES (1, 1e308, 3e38), (1, 1e308, 3e38)") + + def state(on, sql): + _groupvec(pgc_conn, on) + try: + _one(pgc_conn, sql) + return "ok" + except psycopg.Error as e: + return e.sqlstate or "?" + + _groupvec(pgc_conn, True) + fires = "|".join("yes" if MARKER in str(_plan(pgc_conn, f"SELECT k, sum({c}) FROM ovs_col GROUP BY k")) else "no" + for c in ("d", "r")) + expect.text(fires, "yes|yes", "sum(float8) and sum(real) overflow: node fires") + got = "|".join(state(on, f"SELECT k, sum({c}) FROM ovs_col GROUP BY k") + for on in (True, False) for c in ("d", "r")) + _groupvec(pgc_conn, True) + expect.text(got, "22003|22003|22003|22003", "sum(float8) and sum(real) overflow raise 22003, as core does") + + def test_a_gating_where_is_honoured_rather_than_dropped(pgc_conn, expect): """Blocker 2: a pseudoconstant one-time filter was dropped, so a false gate wrongly returned rows. The node cannot honour a one-time filter, so it must From e8a1af59dfeb161c30b1efb6c08537d64489ba89 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 01:20:33 +0000 Subject: [PATCH 2/3] fix: REINDEX reclaims the entries of deleted rows no snapshot can see (#1407) older snapshot still finds a recently deleted row by a new index. But it emitted every deleted row, where heap's build skips a tuple dead to all. 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. The build now also reads each row group's delete marks under a horizon snapshot (xmin = xmax = the oldest non-removable xid, curcid 0), which sees exactly the transactions committed before every running snapshot began, and leaves out a row deleted there. The snapshot is registered: PG 18+ asserts it in the visibility check. am_contracts gains the arm in both twins, red on main (kept:2260992-> 2260992); with the horizon moved to the next xid, the older-snapshot arm goes red instead. limitations.md says when REINDEX keeps an entry. cluster_tests +1. Fixes #1407. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- docs/limitations.md | 4 +++- src/columnar_reader.c | 37 ++++++++++++++++++++++++++++++++ src/columnar_reader.h | 1 + src/columnar_tableam.c | 22 +++++++++++++++++++ test/am_contracts.sh | 18 ++++++++++++++++ test/check_ledger.tsv | 1 + test/check_ledger_budget.txt | 10 +++++++++ test/pytest/TESTS.md | 1 + test/pytest/expected_tests.txt | 4 +++- test/pytest/test_am_contracts.py | 28 ++++++++++++++++++++++++ 10 files changed, 124 insertions(+), 2 deletions(-) diff --git a/docs/limitations.md b/docs/limitations.md index 9bd7666fb..edcaa84cd 100644 --- a/docs/limitations.md +++ b/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. diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 2c9b0d003..6fa69e843 100644 --- a/src/columnar_reader.c +++ b/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) { diff --git a/src/columnar_reader.h b/src/columnar_reader.h index c211ddd1d..5feac316d 100644 --- a/src/columnar_reader.h +++ b/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); diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index d2e0b8101..6c7d94207 100644 --- a/src/columnar_tableam.c +++ b/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; } diff --git a/test/am_contracts.sh b/test/am_contracts.sh index 1ebb8c9a6..900d6a514 100755 --- a/test/am_contracts.sh +++ b/test/am_contracts.sh @@ -309,4 +309,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 diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f5b5c01b4..234896d0a 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1,6 +1,7 @@ am_contracts am_contracts ON CONFLICT DO NOTHING skips the duplicate and inserts the new row 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts ON CONFLICT DO UPDATE is refused as unsupported (0A000), and changes nothing 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts ON CONFLICT DO UPDATE with no conflicting row inserts it 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 +am_contracts am_contracts REINDEX after a committed delete of most rows reclaims their entries, as heap's does 15;16;17;18;19 2026-10-15 the index build indexing every deleted row (main before #1407's fix) am_contracts am_contracts a DO NOTHING racing an uncommitted insert of the same key waits, then skips it 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts a TRUNCATE rolled back at transaction level restores every row 15;16;17;18;19 2026-10-14 TRUNCATE empties the old storage in place (RelationTruncate before the swap) am_contracts am_contracts a TRUNCATE rolled back to a savepoint restores every row, and the transaction goes on 15;16;17;18;19 2026-10-14 TRUNCATE empties the old storage in place (RelationTruncate before the swap) diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 3a800607d..ba141ef3f 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2307,4 +2307,14 @@ suites_not_covered 231 # never 2179 + 93 = 2272 # dated 694 + 1 = 695 # 2272 + 695 == 2967 +# +# #1407, REINDEX reclaims the entries of rows no snapshot sees (jdatcmd): 2272 -> +# 2272. am_contracts gains one check from five-major runs (28/28 on each of 15-19), +# dated 695 + 1 = 696 +# its size where heap's shrank to a tenth. +# +# ledger rows 2967 + 1 = 2968 +# never 2272 + 0 = 2272 +# dated 695 + 1 = 696 +# 2272 + 696 == 2968 checks_never_observed_red 2272 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 0e597cec2..896de9e4e 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -6918,6 +6918,7 @@ THE WITHDRAWAL IS REACHED ONLY BY A RACE. When one session holds a key uncommitt | `test_a_read_inside_an_aborted_inner_block_keeps_the_outer_statements_rows` | a read of the table inside an EXCEPTION block writes out the enclosing INSERT's or DELETE's buffered rows and marks, and keeps them until the block resolves (#1360). On abort the rows, the read's own count and the index's view all match heap; so do a read in a nested block that commits into an aborted one, and a block that deletes a row by index before its read. A block that commits keeps every outer row once. The cost is linear: at twice the rows, the file is under 2.5 times the size; and one read early in a 5000-row insert leaves at most 3 row groups | | `test_an_insert_racing_an_uncommitted_delete_of_its_key` | an insert of a key whose delete (or key-changing update) is uncommitted waits, then fails with 23505 if the delete rolls back and succeeds if it commits: never two rows with the key. Before #1341's fix the rollback left two | | `test_an_index_built_after_a_delete_serves_an_older_snapshot` | a REPEATABLE READ snapshot from before a committed DELETE still finds the row through an index built after it (an Index Scan, asserted), and a new snapshot does not; a UNIQUE index still builds over a key whose other holder is deleted. Before #1341's fix the old snapshot's lookup found 0 | +| `test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees` | after a committed delete of 9 rows in 10, with no older snapshot, `REINDEX` takes the index under half its size, as it does on heap, and the index still finds the 10,000 live rows (#1407) | ## 111. test_objstore_credentials.py: object-store credentials live on a user mapping, and the environment is a privilege diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index a7c06924a..2f717dc17 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -675,4 +675,6 @@ guard_tests 435 # over the cluster files on this tree. # cluster_tests 567 -> 568 for #1341's test_a_float_sum_that_overflows_raises_as_core_does in # test_native_groupagg.py, by collection over the cluster files on this tree. -cluster_tests 568 +# cluster_tests 568 -> 569 for #1407's test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees in +# test_am_contracts.py, by collection over the cluster files on this tree. +cluster_tests 569 diff --git a/test/pytest/test_am_contracts.py b/test/pytest/test_am_contracts.py index ca4a275d7..ce881ea4a 100644 --- a/test/pytest/test_am_contracts.py +++ b/test/pytest/test_am_contracts.py @@ -418,3 +418,31 @@ def test_an_index_built_after_a_delete_serves_an_older_snapshot(pgc_cluster, pgc five = cur.fetchone()[0] cur.execute("RESET enable_seqscan") expect.text(f"{built}|{five}", "ok|1", "a unique index builds over a key whose other holder is a deleted row") + + +def test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees(pgc_conn, expect): + """An index build holds deleted rows only while some snapshot can still see them + (#1407). It held every deleted row, 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. Heap is run alongside as the control.""" + with pgc_conn.cursor() as cur: + for am in ("pgcolumnar", "heap"): + cur.execute(f"CREATE TABLE ix_{am} (a int) USING {am}") + cur.execute(f"INSERT INTO ix_{am} SELECT g FROM generate_series(1, 100000) g") + cur.execute(f"CREATE INDEX ix_{am}_a ON ix_{am} (a)") + cur.execute(f"DELETE FROM ix_{am} WHERE a % 10 <> 0") + + def reclaim(am): + before = _one(pgc_conn, f"SELECT pg_relation_size('ix_{am}_a')") + with pgc_conn.cursor() as cur: + cur.execute(f"REINDEX INDEX ix_{am}_a") + after = _one(pgc_conn, f"SELECT pg_relation_size('ix_{am}_a')") + return "reclaimed" if after < before // 2 else f"kept:{before}->{after}" + + with pgc_conn.cursor() as cur: + col, heap = reclaim("pgcolumnar"), reclaim("heap") + cur.execute("SET enable_seqscan = off") + found = _one(pgc_conn, "SELECT count(*) FROM ix_pgcolumnar WHERE a > 0") + cur.execute("RESET enable_seqscan") + expect.text(f"{col}|{heap}|{found}", "reclaimed|reclaimed|10000", + "REINDEX after a committed delete of most rows reclaims their entries, as heap's does") From 7aa53fa6a28eefd8142f547c350bdcc896d098eb Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 03:12:01 +0000 Subject: [PATCH 3/3] test: am_contracts' older-snapshot arm waits for its events instead of sleeping The arm from #1364 held a REPEATABLE READ snapshot, slept four seconds, and assumed the DELETE and CREATE INDEX finished inside them. On a loaded box they did not: the plan was taken before the index existed, and 'premise: the older snapshot's lookup is an index scan' read 0 on PG16. A 5-second delay before CREATE INDEX reproduces it on PG17 every time. Now a gate session holds an advisory lock. Session 1 takes its snapshot, names itself, and queues for the lock. The script waits for the name, deletes, builds the index, then ends the gate. With the same delay the suite passes 28/28. A new premise in both twins says the older snapshot counted all 10000 rows, so it predates the delete. The arm still goes red when the index build skips deleted rows again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/am_contracts.sh | 28 ++++++++++++++++++++++++++-- test/check_ledger.tsv | 1 + test/check_ledger_budget.txt | 13 ++++++++++++- test/pytest/test_am_contracts.py | 2 ++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/test/am_contracts.sh b/test/am_contracts.sh index 900d6a514..2a7c38bad 100755 --- a/test/am_contracts.sh +++ b/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" \ diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 234896d0a..ecf5fdc39 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -30,6 +30,7 @@ am_contracts am_contracts and the primary key still refuses a duplicate of a res am_contracts am_contracts and the unique index answers the key with that one row 15;16;17;18;19 never - am_contracts am_contracts premise: the conflict table holds ten rows 15;16;17;18;19 never - am_contracts am_contracts premise: the index exists and is valid 15;16;17;18;19 never - +am_contracts am_contracts premise: the older snapshot was taken before the delete 15;16;17;18;19 never - am_contracts am_contracts premise: the older snapshot's lookup is an index scan 15;16;17;18;19 never - am_contracts am_contracts premise: the racing sessions contended for keys 15;16;17;18;19 never - am_contracts am_contracts premise: the truncate table holds 5000 rows 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index ba141ef3f..78e175337 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2317,4 +2317,15 @@ suites_not_covered 231 # never 2272 + 0 = 2272 # dated 695 + 1 = 696 # 2272 + 696 == 2968 -checks_never_observed_red 2272 +# +# am_contracts' older-snapshot arm ordered by events, not sleeps (jdatcmd): 2272 -> +# 2273. One premise enters, that the older snapshot predates the delete, from +# five-major runs (28/28 on each of 15-19). A premise is never dated, by #1236. +# The arm it guards stays dated: an index build that skips deleted rows again +# reddens it on PG17 under the new ordering. +# +# ledger rows 2968 + 1 = 2969 +# never 2272 + 1 = 2273 +# dated 696 + 0 = 696 +# 2273 + 696 == 2969 +checks_never_observed_red 2273 diff --git a/test/pytest/test_am_contracts.py b/test/pytest/test_am_contracts.py index ce881ea4a..5780897d5 100644 --- a/test/pytest/test_am_contracts.py +++ b/test/pytest/test_am_contracts.py @@ -378,6 +378,7 @@ def test_an_index_built_after_a_delete_serves_an_older_snapshot(pgc_cluster, pgc oc.execute(f'SET search_path TO "{schema}"') oc.execute("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ") oc.execute("SELECT count(*) FROM ib") + before = oc.fetchone()[0] with pgc_conn.cursor() as cur: cur.execute("DELETE FROM ib WHERE id = 4242") cur.execute("CREATE INDEX ib_id ON ib (id)") @@ -390,6 +391,7 @@ def test_an_index_built_after_a_delete_serves_an_older_snapshot(pgc_cluster, pgc old.rollback() finally: old.close() + expect.num(before, 10000, "premise: the older snapshot was taken before the delete") expect.text(str(_one(pgc_conn, "SELECT indisvalid FROM pg_index WHERE indexrelid = 'ib_id'::regclass")), "True", "premise: the index exists and is valid") expect.num(len(re.findall(r"Index (Only )?Scan using ib_id", plan)), 1,