Re: row filtering for logical replication

On Thu, Feb 17, 2022, at 3:36 AM, Amit Kapila wrote:
> As there is a new version, I would like to wait for a few more days
> before committing. I am planning to commit this early next week (by
> Tuesday) unless others or I see any more things that can be improved.
Amit, I don't have additional comments or suggestions. Let's move on. Next
topic. :-)

> I would once like to mention the replica identity handling of the
> patch. Right now, (on HEAD) we are not checking the replica identity
> combination at DDL time, they are checked at execution time in
> CheckCmdReplicaIdentity(). This patch follows the same scheme and
> gives an error at the time of update/delete if the table publishes
> update/delete and the publication(s) has a row filter that contains
> non-replica-identity columns. We had earlier thought of handling it at
> DDL time but that won't follow the existing scheme and has a lot of
> complications as explained in emails [1][2]. Do let me know if you see
> any problem here?
IMO it is not an issue that this patch needs to solve. The conclusion of
checking the RI at the DDL time vs execution time is that:

* the current patch just follows the same pattern used in the current logical
replication implementation;
* it is easier to check during execution time (a central point) versus a lot of
combinations for DDL commands;
* the check during DDL time might eventually break if new subcommands are
added;
* the execution time does not have the maintenance burden imposed by new DDL
subcommands;
* we might change the RI check to execute at DDL time if the current
implementation imposes a significant penalty in certain workloads.

Again, it is material for another patch.

Thanks for taking care of a feature that has been discussed for 4 years [1].

[1] https://www.postgresql.org/message-id/CAHE3wggb715X%2BmK_DitLXF25B%3DjE6xyNCH4YOwM860JR7HarGQ%40mail.gmail.com

--
Euler Taveira
EDB https://www.enterprisedb.com/