Re: [COMMITTERS] pgsql: Logical replication

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