Re: Column Filtering in Logical Replication - Mailing list pgsql-hackers

On Sat, Sep 25, 2021 at 1:15 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, Sep 24, 2021 at 6:55 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >
> > On 2021-Sep-23, Amit Kapila wrote:
> >
> > > Alvaro, do you have any thoughts on these proposed grammar changes?
> >
> > Yeah, I think pubobj_name remains a problem in that you don't know its
> > return type -- could be a String or a RangeVar, and the user of that
> > production can't distinguish.  So you're still (unnecessarily, IMV)
> > stashing an object of undetermined type into ->object.
> >
> > I think you should get rid of both pubobj_name and pubobj_expr and do
> > somethine like this:
> >
> > /* FOR TABLE and FOR ALL TABLES IN SCHEMA specifications */
> > PublicationObjSpec:     TABLE ColId
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_TABLE;
> >                                                 $$->rangevar = makeRangeVarFromQualifiedName($1, NULL, @1,
yyscanner);
> >                                         }
> >                         | TABLE ColId indirection
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_TABLE;
> >                                                 $$->rangevar = makeRangeVarFromQualifiedName($1, $2, @1,
yyscanner);
> >                                         }
> >                         | ALL TABLES IN_P SCHEMA ColId
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_REL_IN_SCHEMA;
> >                                                 $$->name = $4;
> >                                         }
> >                         | ALL TABLES IN_P SCHEMA CURRENT_SCHEMA /* XXX should this be "IN_P CURRENT_SCHEMA"? */
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_CURRSCHEMA;
> >                                                 $$->name = $4;
> >                                         }
> >                         | ColId
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->name = $1;
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_CONTINUATION;
> >                                         }
> >                         | ColId indirection
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->rangevar = makeRangeVarFromQualifiedName($1, $2, @1,
yyscanner);
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_CONTINUATION;
> >                                         }
> >                         | CURRENT_SCHEMA
> >                                         {
> >                                                 $$ = makeNode(PublicationObjSpec);
> >                                                 $$->pubobjtype = PUBLICATIONOBJ_CURRSCHEMA;
> >                                         }
> >                 ;
>
> Apart from the issue that Hou San pointed, I found one issue with
> introduction of PUBLICATIONOBJ_CURRSCHEMA, I was not able to
> differentiate if it is table or schema in the following cases:
> CREATE PUBLICATION pub1 FOR ALL TABLES IN SCHEMA CURRENT_SCHEMA;
> CREATE PUBLICATION pub1 FOR ALL TABLES IN SCHEMA sch1, CURRENT_SCHEMA;
> CREATE PUBLICATION pub1 FOR table t1, CURRENT_SCHEMA;
> The differentiation is required to differentiate and add a schema or a table.
>

I am not sure what makes you say that we can't distinguish the above
cases when there is already a separate rule for CURRENT_SCHEMA? I
think you can distinguish by tracking the previous objects as we are
already doing in the patch. But one thing that is not clear to me is
is the reason to introduce a new type PUBLICATIONOBJ_CURRSCHEMA when
we use PUBLICATIONOBJ_REL_IN_SCHEMA and PUBLICATIONOBJ_CONTINUATION to
distinguish all cases of CURRENT_SCHEMA. Alvaro might have something
in mind for this which is not apparent and that might have caused
confusion to you as well?

-- 
With Regards,
Amit Kapila.