Hi Boyu,

Did more experiment:

Decoded output for db1 is identical, and test_decoding, subscription and
recovery all pass.  The idea is sound: skipping a foreign database's
invalidations is a no-op anyway, since LocalExecuteInvalidationMessage()
already filters them by dbId.

On Shlok's question, your answer is right -- the extra messages are from
the rolled-back savepoint, and not distributing a catalog change that
never committed is harmless.  parsed->msgs is the correct set to test.

Smaller things for a v2:

1. InvalidationsTouchSharedCatalog() should switch on msg->id with a
   default: elog(ERROR, ...).  A new message type would otherwise be
   silently treated as "not shared".  The SHAREDINVALSMGR_ID branch is
   dead code.

2. The dbId test contradicts DecodeTXNNeedSkip() just below, which
   treats InvalidOid as "don't assume another database".

3. "distribute" controls both the rebuild and the distribution; please
   rename or split it, and put the justification in a comment rather
   than only in the commit message.

4. Tests: a TAP test with two databases doing shared-catalog DDL in db2
   while a transaction is in progress in db1, plus Shlok's savepoint
   case.

Thanks,
Shihao

Reply via email to