RE: In-placre persistance change of a relation

Hi Kyotaro,

> At Tue, 21 Dec 2021 13:07:28 +0000, Jakub Wartak
> <Jakub(dot)Wartak(at)tomtom(dot)com> wrote in
> > So what's suspicious is that 122880 -> 0 file size truncation. I've
> > investigated WAL and it seems to contain TRUNCATE records after logged
> FPI images, so when the crash recovery would kick in it probably clears this
> table (while it shouldn't).
>
> Darn.. It is too silly that I wrongly issued truncate records for the target
> relation of the function (rel) instaed of the relation on which we're currently
> operating at that time (r).
>
> [..]
> The following fix works.

Cool, I have verified basic stuff that was coming to my mind, now even cfbot is happy with v11, You should happy too I hope :)

> I made another change in this version. Previously only btree among all index
> AMs was processed in the in-place manner. In this version we do that all
> AMs except GiST. Maybe if gistGetFakeLSN behaved the same way for
> permanent and unlogged indexes, we could skip index rebuild in exchange of
> some extra WAL records emitted while it is unlogged.

I think there's slight omission:

-- unlogged table -> logged with GiST:
DROP TABLE IF EXISTS testcase;
CREATE UNLOGGED TABLE testcase(geom geometry not null);
CREATE INDEX idx_testcase_gist ON testcase USING gist(geom);
INSERT INTO testcase(geom) SELECT ST_Buffer(ST_SetSRID(ST_MakePoint(-1.0, 2.0),4326), 0.0001);
ALTER TABLE testcase SET LOGGED;

-- crashes with:
(gdb) where
#0 reindex_index (indexId=indexId(at)entry=65541, skip_constraint_checks=skip_constraint_checks(at)entry=true, persistence=persistence(at)entry=112 'p', params=params(at)entry=0x0) at index.c:3521
#1 0x000000000062f494 in RelationChangePersistence (tab=tab(at)entry=0x1947258, persistence=112 'p', lockmode=lockmode(at)entry=8) at tablecmds.c:5434
#2 0x0000000000642819 in ATRewriteTables (context=0x7ffc19c04520, lockmode=<optimized out>, wqueue=0x7ffc19c04388, parsetree=0x1925ec8) at tablecmds.c:5644
[..]
#10 0x00000000007f078f in exec_simple_query (query_string=0x1925340 "ALTER TABLE testcase SET LOGGED;") at postgres.c:1215

apparently reindex_index() params cannot be NULL - the same happens with switching persistent
table to unlogged one too (with GiST).

I'll also try to give another shot to the patch early next year - as we are starting long Christmas/holiday break here
- with verifying WAL for GiST and more advanced setup (more crashes, and standby/archiving/barman to see
how it's possible to use wal_level=minimal <-> replica transitions).

-J.