Re: row filtering for logical replication - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: row filtering for logical replication
Date
Msg-id CAA4eK1J3qrpymnGbha6bo3v8zDoWAWKt5kMYrhGtwEVzOsCstw@mail.gmail.com
Whole thread Raw
In response to RE: row filtering for logical replication  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
Responses Re: row filtering for logical replication
Re: row filtering for logical replication
List pgsql-hackers
On Sat, Jan 15, 2022 at 12:00 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Friday, January 14, 2022 7:31 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Thu, Jan 13, 2022 at 6:46 PM houzj.fnst@fujitsu.com
> >
> > 9.
> > --- a/src/backend/replication/logical/proto.c
> > +++ b/src/backend/replication/logical/proto.c
> > @@ -15,6 +15,7 @@
> >  #include "access/sysattr.h"
> >  #include "catalog/pg_namespace.h"
> >  #include "catalog/pg_type.h"
> > +#include "executor/executor.h"
> >
> > Do we really need this include now? Please check includes in other
> > files as well and remove if anything is not required.
> >
...
....
>
> Thanks for the comments.
> Attach the V65 patch set which addressed the above comments and Peter's comments[1].

The above comment (#9) doesn't seem to be addressed. Also, please
check other includes as well. I find below include also unnecessary.

--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
...
...
+#include "nodes/nodeFuncs.h"

Some other comments:
==================
1.
/*
+ * If we know everything is replicated and some columns are not part
+ * of replica identity, there is no point to check for other
+ * publications.
+ */
+ if (pubactions.pubinsert && pubactions.pubupdate &&
+ pubactions.pubdelete && pubactions.pubtruncate &&
+ (!validate_rowfilter || !rfcol_valid))
break;

Why do we need to continue for other publications after we find there
is an invalid column in row_filter?

2.
* For initial synchronization, row filtering can be ignored in 2 cases:
+ *
+ * 1) one of the subscribed publications has puballtables set to true
+ *
+ * 2) one of the subscribed publications is declared as ALL TABLES IN
+ * SCHEMA that includes this relation

Isn't there one more case (when one of the publications has a table
without any filter) where row filtering be ignored? I see that point
being mentioned later but it makes things unclear. I have tried to
make things clear in the attached.

Apart from the above, I have made a few other cosmetic changes atop
v65-0001*.patch. Kindly review and merge into the main patch if you
are okay with these changes.

-- 
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: Jeff Davis
Date:
Subject: Re: Logical insert/update/delete WAL records for custom table AMs
Next
From: Daniel Gustafsson
Date:
Subject: Re: New developer papercut - Makefile references INSTALL