fix: an in-place TRUNCATE must clear the projection's storage too (#896)

[OffgridwithJD]

Closes #896.

TRUNCATE then INSERT in one transaction failed with a row_group_pkey violation on any columnar table carrying a projection, and rolled the whole transaction back.

The condition, and the two things I got wrong when I filed it

I originally wrote that whether the table pre-exists makes no difference. That is the decisive condition, and I had it backwards — the reproduction put every statement into a single psql -c, which psql sends as one implicit transaction, so the table I described as pre-existing did not. I also guessed the cause was a missing forget for the projection's write state; projection writers live inside the base relation's write state as writeState->projWriters, keyed by the base relid, so the existing forget already drops them.

The table must be created in the same transaction as the TRUNCATE. Six arms, each changing one thing:

arm	result
CREATE separate, rest in one transaction	OK
CREATE+INSERT+add_projection together, TRUNCATE+INSERT separate	OK
everything together, explicit BEGIN/COMMIT	ERROR
everything together, no explicit transaction block	ERROR
everything together, no add_projection (control)	OK
everything together, no write after the TRUNCATE (control)	OK

Why, measured

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:

same transaction as CREATE:   before relfilenode=16567 storage=10000000000
                              after  relfilenode=16567 storage=10000000000
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 the truncate should have removed. The colliding storage id is the projection's own.

The fix, and what it deliberately does not do

The callback now clears each projection's storage as well as the base.

The pgcolumnar.projection rows 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 is being truncated away.

The arm

In test/truncate_cleanup.sh, which already covers the rewrite path's projection leak — this is the same leak reached the other way. The suite already carried a passing control for the same transaction shape without a projection, which is what bounded the cause.

It asserts one row group holding 500 rows rather than a group number. Measured after the fix, 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. My first version of the check did exactly that.

Verification

Red then green, with the fix reverted and restored around the red run and the restored file verified byte-identical rather than trusted to a git checkout:

without the fix:  17 passed + 8 failed
with the fix:     25 passed + 0 failed

Full matrix on both majors at 7e17494, rebased onto 2e543f27 so #892 is present:

PG18   245 of 247 suites ran (2 skipped, 0 incomplete)   zero FAIL
PG19   247 of 247 suites ran (0 skipped, 0 incomplete)   zero FAIL
ALL VERSIONS PASSED

Because #892 landed in the same file and now makes TRUNCATE trigger the projection repair, I re-ran both interacting suites after the rebase before spending the matrix: truncate_cleanup 25/25 and projection_rewrite 84/84.

[jdatcmd]

Approving at 7e174945. The fix is right, the arm is load-bearing, and I checked both by
running rather than reading.

The removal proof

Both arms through the same build directory, so the .so fingerprints are a source difference
rather than a build-path one:

fix present        .so 5efb9856f12f    25 passed +  0 failed
clear neutered     .so 97dccc2965fd    17 passed +  8 failed

The mutation kept the loop and neutered only the delete, and it is asserted at source — the marker
present, and zero PgColumnarDeleteMetadata(p->projStorageId) calls remaining within
pgcolumnar_relation_nontransactional_truncate rather than in the file, since
pgcolumnar_delete_storage_tree at :886 has its own.

The eight reds are the right ones, and the first is the one that matters:

FAIL  PREMISE the in-place transaction committed: got [1] want [0]
FAIL  PREMISE the table exists after it: got [0] want [1]
FAIL  the projection storage holds exactly one row group after the truncate: got [] want [1]

The transaction rolls back — #896's actual symptom — rather than some downstream count moving.

A note on my first attempt, because it says something about the gate. My first mutation deleted
the whole foreach, which left lc unused, and the build was rejected before the suite ran:
warning: unused variable 'lc' / rebuild: 1 compiler warning(s) -- the matrix gate rejects these. A reviewer who took that as "the arm went red" would have recorded a removal proof that
never executed. Worth stating because a mutation that fails to build is the quiet way a removal
proof lies.

The composed tree

This branch is 7 commits behind main — it predates #899, which changed columnar_vacuum.c
substantially and added the clustering verbs. mergeStateStatus is CLEAN, but a green PR describes
its branch and not the composition, so I merged 6da7c3f with 7e174945 locally (composed
61d19ee2) and ran the tree that would actually exist. COPT=-Werror, 0 warnings, 0 FAIL
lines:

truncate_cleanup    25    hilbert_cluster   181    alter_am_cleanup  45
projection_rewrite  84    hilbert_curve     184    drop_cleanup       8
projections         73    native_cluster     11

projection_rewrite and projections are #892's, hilbert_* are #899's, and alter_am_cleanup
and drop_cleanup are the other two suites that exercise the storage-teardown paths this change
sits in.

What I checked in the code, and the part I would have got wrong

The guard if (p->projStorageId != storageId) skips the base projection, whose projectionId is 0
and whose storage is the table's — so the base is not deleted twice. That matches the pattern
pgcolumnar_compact_relation already uses.

The comment explaining why this is not pgcolumnar_delete_storage_tree is the part I would
have got wrong unprompted, and it is the whole distinction: that function also deletes the
pgcolumnar.projection rows, which is right for a rewrite because the base storage id changes and
the rows are re-recorded under the new one. Here the metapage keeps its id, so the rows still
describe this relation correctly and only their content is being truncated. Deleting them would
have turned a row_group_pkey violation into #876's 42704.

Listing the projections before PgColumnarDeleteMetadata(storageId) is not strictly required,
since that call does not touch projection rows, but it is the right order to write and costs
nothing.

On the issue write-up

Correcting #896 publicly — that the table must be created in the same transaction, and that your
"pre-existing makes no difference" claim was backwards because psql -c sends the whole string as
one implicit transaction — is worth more than the fix. The six arms each changing one thing, with
the no-projection and no-write-after controls, are what make the condition a measurement rather
than a story. And the relfilenode/storage-id table showing the in-place case keeping both is the
evidence the whole diagnosis rests on.

One follow-on, not for this PR: PgColumnarForgetWriteStateForRelation stays, and your note that
projection writers live inside the base write state as writeState->projWriters keyed by the base
relid is the reason it is sufficient. That is worth being in a comment somewhere, because the next
person to read this path will have the same wrong hypothesis you and I both started with.