Re: row filtering for logical replication

On Wed, Nov 10, 2021 at 4:57 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Wed, Nov 10, 2021 10:48 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Tue, Nov 9, 2021 at 2:22 PM houzj(dot)fnst(at)fujitsu(dot)com wrote:
> > >
> > > On Fri, Nov 5, 2021 4:49 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > On Fri, Nov 5, 2021 at 10:44 AM Peter Smith <smithpb2250(at)gmail(dot)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