Re: [COMMITTERS] pgsql: Logical replication - Mailing list pgsql-committers

Hi,

On 2017-01-20 14:06:18 +0000, Peter Eisentraut wrote:
> Logical replication
> 
> - Add PUBLICATION catalogs and DDL
> - Add SUBSCRIPTION catalog and DDL
> - Define logical replication protocol and output plugin
> - Add logical replication workers

I know I mentioned this before, but I just hit it again. Please split
up your commits in more logical chunks. This has a *LOT* of largely
unrelated pieces. Lots of them without so much as a comment about why
they have been changed.  The above certainly doesn't explain why the
*generic* cache invalidation code was affected:

@@ -509,8 +514,10 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId)
    /*
     * If the relation being invalidated is one of those cached in the local
     * relcache init file, mark that we need to zap that file at commit.
+    * Same is true when we are invalidating whole relcache.
     */
-   if (OidIsValid(dbId) && RelationIdIsInInitFile(relId))
+   if (OidIsValid(dbId) &&
+       (RelationIdIsInInitFile(relId) || relId == InvalidOid))
        transInvalInfo->RelcacheInitFileInval = true;
 }

I mean, like, seriously?  What is this stuff about?

I kinda guess the motivating factor is:

/*
 * CacheInvalidateRelcacheAll
 *        Register invalidation of the whole relcache at the end of command.
 *
 * This is used by alter publication as changes in publications may affect
 * large number of tables.
 */
void
CacheInvalidateRelcacheAll(void)
{
    PrepareInvalidationState();

    RegisterRelcacheInvalidation(InvalidOid, InvalidOid);
}

but that doesn't explain why it's not tied to a database?  Nor why the
relcache init file is now invalidated when there's a publication change?

Greetings,

Andres Freund