RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns

On Thursday, July 14, 2022 10:31 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> I've attached an updated patch that incorporated comments from Amit and Shi.
Hi,

Minor comments for v4.

(1) typo in the commit message

"When decoding a COMMIT record, we check both the list and the ReorderBuffer to see if
if the transaction has modified catalogs."

There are two 'if's in succession in the last sentence of the second paragraph.

(2) The header comment for the spec test

+# Test that decoding only the commit record of the transaction that have
+# catalog-changed.

Rewording of this part looks required, because "test that ... " requires a complete sentence
after that, right ?

(3) SnapBuildRestore

snapshot_not_interesting:
if (ondisk.builder.committed.xip != NULL)
pfree(ondisk.builder.committed.xip);
return false;
}

Do we need to add pfree for ondisk.builder.catchange.xip after the 'snapshot_not_interesting' label ?

(4) SnapBuildPurgeOlderTxn

+ elog(DEBUG3, "purged catalog modifying transactions from %d to %d",
+ (uint32) builder->catchange.xcnt, surviving_xids);

To make this part more aligned with existing codes,
probably we can have a look at another elog for debug in the same function.

We should use %u for casted xcnt & surviving_xids,
while adding a format for xmin if necessary ?

Best Regards,
Takamichi Osumi