From 7e174945a0b7443f6e0d20f15da8ad98fa8f0f9c Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 03:49:35 +0000 Subject: [PATCH] fix: an in-place TRUNCATE must clear the projection's storage too (#896) TRUNCATE then INSERT in one transaction failed with a row_group_pkey violation on any columnar table carrying a projection, and rolled the transaction back. PostgreSQL truncates a relation created in the current transaction in place: a rollback discards the relation anyway, so there is no reason to mint a new relfilenode. Measured on 18.4: created in the same transaction: before relfilenode=16567 storage=10000000000 after relfilenode=16567 storage=10000000000 created in an earlier transaction: before relfilenode=16570 storage=10000000001 after relfilenode=16573 storage=10000000002 So relation_set_new_filelocator never runs and nothing retires the projection's storage. pgcolumnar_relation_nontransactional_truncate cleared only the base: row_group before : storage 10000000000 groups 1 (base) row_group before : storage 10000000001 groups 1 (projection) row_group AFTER : storage 10000000001 groups 1 <- base cleared, projection kept and the next write to the projection collided with what was left. The projection rows themselves are kept, which is why this is not pgcolumnar_delete_storage_tree. That function also deletes them, and is right to for a rewrite: the base storage id changes there, so the rows are retired and re-recorded under the new id. Here the metapage keeps its storage id, so the rows still describe this relation correctly and only their content goes. The arm is in test/truncate_cleanup.sh, which already covers the rewrite path's projection leak; this is the same leak reached the other way. It asserts one row group holding 500 rows rather than a group NUMBER: measured, the base gets group 1 and the projection group 2, because PgColumnarResetMetapage resets the base's counter and a projection's storage has no metapage to reset. That asymmetry is not a defect and an arm pinning the number would fail for the wrong reason. Red then green, with the fix reverted and restored around the red run and the restored file verified byte-identical: 17 passed + 8 failed without it, 25 passed + 0 failed with it. The suite already carried a passing control for the same transaction shape WITHOUT a projection, which is what bounded the cause. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 28 +++++++++++++++ src/columnar_tableam.c | 28 +++++++++++++++ test/truncate_cleanup.sh | 77 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aba729d..9d9f3e9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,34 @@ true until the next version shipped. the walk to use the named relation instead of each descendant takes `test/projection_rename_restore.sh` from 8 passed to 6 passed and 2 failed. +- An in-place `TRUNCATE` now clears each projection's storage as well as the base + (#896). + + PostgreSQL truncates a relation created in the current transaction in place, + because a rollback discards the relation anyway. Measured on 18.4: the + relfilenode and the storage id are both unchanged across such a `TRUNCATE`, + where a table from an earlier transaction gets fresh ones. So + `relation_set_new_filelocator` never runs, and the teardown that retires a + projection's storage never happens. + + `pgcolumnar_relation_nontransactional_truncate` cleared only the base storage. + The projection's row groups survived, and the next write to the projection + collided with them: + + ERROR: duplicate key value violates unique constraint "row_group_pkey" + DETAIL: Key (storage_id, group_number)=(10000000001, 2) already exists. + + The whole transaction rolled back, so `TRUNCATE` and reload in one transaction + was unavailable on any table carrying a projection. It needed four things + together: the table created in the same transaction, a projection, the + `TRUNCATE`, and a write after it. Each was isolated by a control. + + **The `pgcolumnar.projection` rows are kept, deliberately.** This is not + `pgcolumnar_delete_storage_tree`, which also deletes them. That is right for a + rewrite, where the base storage id changes and the rows are re-recorded under + the new one. Here the metapage keeps its storage id, so the rows still describe + this relation correctly; only their content is being truncated away. + - `ALTER TABLE ... DROP COLUMN` is now refused when a projection depends on the column, instead of leaving the table unreadable (#891). diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index c18b38ca..b754fcc2 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -958,9 +958,37 @@ static void pgcolumnar_relation_nontransactional_truncate(Relation rel) { uint64 storageId = PgColumnarStorageId(rel); + List *projs = PgColumnarListProjections(storageId); + ListCell *lc; PgColumnarDeleteMetadata(storageId); + /* + * A projection keeps its own storage id, so clearing the base storage is + * not enough. This path cleared only the base and left every projection's + * row groups in place; the next write to the projection then collided with + * them (#896): + * + * ERROR: duplicate key value violates unique constraint "row_group_pkey" + * DETAIL: Key (storage_id, group_number)=(10000000001, 2) already exists. + * + * The PROJECTION ROWS THEMSELVES STAY, which is why this is not + * pgcolumnar_delete_storage_tree. That function also deletes the + * pgcolumnar.projection rows, and it is right to for a rewrite: the base + * storage id changes there, so the rows are retired and re-recorded under + * the new id. Here the metapage keeps its storage id, so the projection + * rows still describe this relation correctly. Only their CONTENT is being + * truncated away. + */ + foreach(lc, projs) + { + PgColumnarProjection *p = (PgColumnarProjection *) lfirst(lc); + + if (p->projStorageId != storageId) + PgColumnarDeleteMetadata(p->projStorageId); + } + list_free(projs); + /* * The same stale-write-state hazard as the rewrite path above, reached the * other way: ExecuteTruncateGuts calls heap_truncate_one_rel, and so this diff --git a/test/truncate_cleanup.sh b/test/truncate_cleanup.sh index e670d447..8896163e 100755 --- a/test/truncate_cleanup.sh +++ b/test/truncate_cleanup.sh @@ -197,4 +197,81 @@ check "a full rewrite keeps every row, and they are still readable" \ "5000/5000/5000" psql_run "DROP TABLE tc_rw;" +# --------------------------------------------------------------------------- +# THE IN-PLACE TRUNCATE, which is a different code path from every arm above. +# +# PostgreSQL truncates a relation created in the CURRENT transaction in place: +# a rollback discards the whole relation, so there is no reason to mint a new +# relfilenode. Measured on 18.4: +# +# created in the same transaction: before relfilenode=16567 storage=10000000000 +# after relfilenode=16567 storage=10000000000 +# created in an earlier transaction: before relfilenode=16570 storage=10000000001 +# after relfilenode=16573 storage=10000000002 +# +# So pgcolumnar_relation_set_new_filelocator never runs, and the teardown the +# arms above exercise never happens. The AM gets +# pgcolumnar_relation_nontransactional_truncate instead, which cleared the BASE +# storage and left the projection's own storage untouched: +# +# row_group before : storage 10000000000 groups 1 (base) +# row_group before : storage 10000000001 groups 1 (projection) +# row_group AFTER : storage 10000000001 groups 1 <-- base cleared, projection kept +# +# The next write to the projection then collided with the row groups the +# truncate should have removed: +# +# ERROR: duplicate key value violates unique constraint "row_group_pkey" +# DETAIL: Key (storage_id, group_number)=(10000000001, 2) already exists. +# +# The write AFTER the truncate is what makes it observable, so this arm has one. +# Without it the leaked rows sit there and every check still passes (#896). +echo "-- an in-place TRUNCATE must clear the projection's storage too (#896)" +psql_run "CREATE TABLE tcl_inplace_pre (id int);" >/dev/null +inplace_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" \ + -U postgres -d "$PGC_DB" -v ON_ERROR_STOP=0 -qAt -c " +BEGIN; +CREATE TABLE tcl_inplace (id int, a int, b text) USING pgcolumnar; +INSERT INTO tcl_inplace SELECT g, g % 50, 'b' || g FROM generate_series(1, 2000) g; +SELECT pgcolumnar.add_projection('tcl_inplace','tcl_ip',ARRAY['a','b'],ARRAY['a']); +TRUNCATE tcl_inplace; +INSERT INTO tcl_inplace SELECT g, g % 50, 'c' || g FROM generate_series(1, 500) g; +COMMIT;" 2>&1)" + +# PREMISE: the transaction has to have committed, or every check below is +# vacuous -- a rolled-back transaction leaves no table and no rows to count. +check "PREMISE the in-place transaction committed" \ + "$(printf '%s\n' "$inplace_out" | grep -c 'row_group_pkey')" "0" +check "PREMISE the table exists after it" \ + "$(q "SELECT count(*) FROM pg_class WHERE relname = 'tcl_inplace';" | tail -1)" "1" +check "the rows written after the in-place truncate are all there" \ + "$(q "SELECT count(*) FROM tcl_inplace;" | tail -1)" "500" +check "and the projection reads them back" \ + "$(q "SELECT count(*) FROM pgcolumnar.read_projection('tcl_inplace','tcl_ip');" | tail -1)" "500" +check "the projection agrees with the base table" \ + "$(pgc_set_hash "SELECT pgcolumnar.read_projection('tcl_inplace','tcl_ip')")" \ + "$(pgc_set_hash "SELECT a::text||'|'||b FROM tcl_inplace")" +# The leak itself, counted directly rather than inferred from the error. The +# projection's storage must describe the 500 rows written AFTER the truncate and +# nothing else. Before the fix it held the pre-truncate group as well, which is +# what the next write collided with. +# +# Asserted as one group holding 500 rows, not as a group NUMBER: measured after +# the fix, the base gets group 1 and the projection gets group 2, because +# PgColumnarResetMetapage resets the base's counter and the projection's storage +# has no metapage to reset. That asymmetry is not a defect and an arm that +# pinned the number would fail for the wrong reason. +inplace_proj_storage="$(q "SELECT proj_storage_id FROM pgcolumnar.projection + WHERE storage_id = pgcolumnar.get_storage_id('tcl_inplace'::regclass) + AND projection_id > 0;" | tail -1)" +check "PREMISE the projection has its own storage, distinct from the base" \ + "$(awk -v p="$inplace_proj_storage" -v b="$(q "SELECT pgcolumnar.get_storage_id('tcl_inplace'::regclass);" | tail -1)" \ + 'BEGIN { print (p != "" && p != b) ? "yes" : "no" }')" "yes" +check "the projection storage holds exactly one row group after the truncate" \ + "$(q "SELECT count(*) FROM pgcolumnar.row_group + WHERE storage_id = $inplace_proj_storage;" | tail -1)" "1" +check "and that group describes only the rows written after it" \ + "$(q "SELECT coalesce(sum(row_count),0) FROM pgcolumnar.row_group + WHERE storage_id = $inplace_proj_storage;" | tail -1)" "500" + pgc_summary