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