Re: row filtering for logical replication

On Wed, Nov 10, 2021 at 4:57 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Wed, Nov 10, 2021 10:48 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Tue, Nov 9, 2021 at 2:22 PM houzj.fnst@fujitsu.com wrote:
> > >
> > > On Fri, Nov 5, 2021 4:49 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > On Fri, Nov 5, 2021 at 10:44 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > > > >
> > > > > PSA new set of v37* patches.
> > > > 3.
> > > > - | ColId
> > > > + | ColId OptWhereClause
> > > >   {
> > > >   $$ = makeNode(PublicationObjSpec);
> > > >   $$->pubobjtype = PUBLICATIONOBJ_CONTINUATION;
> > > > - $$->name = $1;
> > > > + if ($2)
> > > > + {
> > > > + $$->pubtable = makeNode(PublicationTable); $$->pubtable->relation
> > > > + = makeRangeVar(NULL, $1, @1); $$->pubtable->whereClause = $2; }
> > > > + else { $$->name = $1; }
> > > >
> > > > Again this doesn't appear to be the right way. I think this should
> > > > be handled at a later point.
> > >
> > > I think the difficulty to handle this at a later point is that we need
> > > to make sure we don't lose the whereclause. Currently, we can only
> > > save the whereclause in PublicationTable structure and the
> > > PublicationTable is only used for TABLE, but '| ColId' can be used for
> > > either a SCHEMA or TABLE. We cannot distinguish the actual type at
> > > this stage, so we always need to save the whereclause if it's NOT NULL.
> > >
> >
> > I see your point. But, I think we can add some comments here indicating that
> > the user might have mistakenly given where clause with some schema which we
> > will identify later and give an appropriate error. Then, in
> > preprocess_pubobj_list(), identify if the user has given the where clause with
> > schema name and give an appropriate error.
> >
>
> OK, IIRC, in this approach, we need to set both $$->name and $$->pubtable in
> '| ColId OptWhereClause'. And In preprocess_pubobj_list, we can add some check
> if both name and pubtable is NOT NULL.
>
> the grammar code could be:
>
> | ColId OptWhereClause
> {
>         $$ = makeNode(PublicationObjSpec);
>         $$->pubobjtype = PUBLICATIONOBJ_CONTINUATION;
>
>         $$->name = $1;
> +       /* xxx */
> +       $$->pubtable = makeNode(PublicationTable);
> +       $$->pubtable->relation = makeRangeVar(NULL, $1, @1);
> +       $$->pubtable->whereClause = $2;
>         $$->location = @1;
> }
>
> preprocess_pubobj_list
> ...
> else if (pubobj->pubobjtype == PUBLICATIONOBJ_REL_IN_SCHEMA ||
> pubobj->pubobjtype == PUBLICATIONOBJ_CURRSCHEMA)
> {
>     ...
> +    if (pubobj->name &&
> +        (!pubobj->pubtable || !pubobj->pubtable->whereClause))
>             pubobj->pubobjtype = PUBLICATIONOBJ_REL_IN_SCHEMA;
>     else if (!pubobj->name && !pubobj->pubtable)
>             pubobj->pubobjtype = PUBLICATIONOBJ_CURRSCHEMA;
>     else
>             ereport(ERROR,
>                             errcode(ERRCODE_SYNTAX_ERROR),
>                             errmsg("invalid schema name at or near"),
>                             parser_errposition(pubobj->location));
> }
>

Hi Hou-san. Actually, I have already implemented this part according
to my understanding of Amit's suggestion and it seems to be working
well.

Please wait for v39-0001, then feel free to post review comments about
it if you think there are still problems.

------
Kind Regards,
Peter Smith.
Fujitsu Australia