RE: row filtering for logical replication

On Wednesday, January 5, 2022 2:01 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> On Tue, Jan 4, 2022 at 9:58 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > Here is the v58* patch set:
> >
> > Main changes from v57* are
> > 1. Couple of review comments fixed
> >
> > ~~
> >
> > Review comments (details)
> > =========================
> >
> > v58-0001 (main)
> > - PG docs updated as suggested [Alvaro, Euler 26/12]
> >
> > v58-0002 (new/old tuple)
> > - pgputput_row_filter_init refactored as suggested [Wangw 30/12] #3
> > - re-ran pgindent
> >
> > v58-0003 (tab, dump)
> > - no change
> >
> > v58-0004 (refactor transformations)
> > - minor changes to commit message
>
> Few comments:

Thanks for the comments!

> 1) We could include namespace names along with the relation to make it more
> clear to the user if the user had specified tables having same table names from
> different schemas:

Since most of the error message in publicationcmd.c and pg_publication.c
doesn't include include namespace names along with the relation,
I am not sure is it necessary to add this. So, I didn’t change this in the patch.

> 5) This log will be logged for each tuple, if there are millions of records it will
> get logged millions of times, we could remove it:
> + /* update requires a new tuple */
> + Assert(newtuple);
> +
> + elog(DEBUG3, "table \"%s.%s\" has row filter",
> +
> get_namespace_name(get_rel_namespace(RelationGetRelid(relation))),
> + get_rel_name(relation->rd_id));

Since the message is logged only in DEBUG3 and could be useful for some
debugging purpose, so I didn't remove this in the new version patch.

Best regards,
Hou zj