Hi shihao,
Thanks for the review. Attached is v2 of the patch.
> 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.
Yes, I choose a if-else style which follows the LocalExecuteInvalidationMessage().
> 2. The dbId test contradicts DecodeTXNNeedSkip() just below, which
> treats InvalidOid as "don't assume another database".
Yes, (dbId == InvalidOid) is now excluded.
> 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.
"distribute" is renamed to "catalog_changes_relevant" to express the exact
meaning. Also, the code to "build a historic snapshot" in SnapBuildCommitTxn()
is now fully controlled by catalog_changes_relevant. I think this is clearer way.
> 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.
A new test is added.
Regards,
Boyu Yang