Re: Proposal: Conflict log history table for Logical Replication

> Here is the rebased version of the upgrade and \dRs patch which is
> present in v45-0003 and v45-0004. There is no change in v45-0001 and
> v45-0002, they are the same patch as in [1].
> [1] - https://www.postgresql.org/message-id/CAFiTN-vWZR9%2BU-kg6d2L3J0WGr6fqESnjah8vrguVCmpEG7SMA%40mail.gmail.com
>

A few comments:

1)
We have introduced errors like these:

+ errdetail("Conflict schema modifications are currently disallowed.")));
+ errmsg("cannot move objects into or out of the conflict schema")));

But here:

{
/* Can't be system namespace */
if (IsCatalogNamespace(schemaid) || IsToastNamespace(schemaid) ||
IsConflictLogTableNamespace(schemaid))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot add schema \"%s\" to publication",
get_namespace_name(schemaid)),
errdetail("This operation is not supported for system schemas.")));

We have included conflict-schema in system schemas, I feel here too,
we shall make a new condition and error mentioning 'conflict schema'.
Irrespective of how TOAST-scehma is handled here, we can have
cosnsitent handling at-least for new conflcit-schema.

2)
+#include "catalog/dependency.h"
+#include "commands/subscriptioncmds.h"

conflict.c cimpiles without above 2 inclusions.


v45-0002:
3)
old name (pg_conflict_log_for_subid_16396) referred in commit message of 002

4)
Since ProcessPendingConflictLogTuple() is called in CATCH-block at
multiple places, it would be better if
ProcessPendingConflictLogTuple() had its own try-catch block. This
block could log/WARN CLT-insertion errors, clear them, and allow the
caller's original error to remain. In rare-scenarios, we may end up
overriding the actual error with a CLT insertion error.

~~
Reviewing more..

thanks
Shveta