> While further testing this feature, I realized that ProcessSingleRelationFork()
> unconditionally called log_newpage_buffer() for every page of every relation
> during pg_enable_data_checksums(). This included unlogged relations,
> which by definition never generate WAL for data changes and are reset to their
> init fork on any recovery.
I've tested your patch, and also expanded the test you wrote a little with a
persistence change.
I tested the patches, it mostly looks good.
I've a small concern in 0001. The new guard uses only RelationNeedsWAL(reln),
but ProcessSingleRelationByOid() iterates all forks. For unlogged relations,
the init fork is special, there are several existing call sites that preserve
WAL for INIT_FORKNUM, for example using
RelationNeedsWAL(rel) || forknum == INIT_FORKNUM
and catalog/storage.c notes that unlogged init forks need WAL and sync.
So I think the condition in ProcessSingleRelationFork() should preserve the
init-fork case, e.g.
if (RelationNeedsWAL(reln) || forkNum == INIT_FORKNUM)
log_newpage_buffer(buf, false);
It would also be good to extend the new test so it exercises a non-empty init
fork, perhaps with an unlogged table that has an index, and then verifies the
standby/recovery behavior.
0002 and 0003 look good to me.
Regards,
Ayush
