On Mon, Aug 23, 2021 at 11:16 PM vignesh C <vignes...@gmail.com> wrote:
>
> On Tue, Aug 17, 2021 at 6:55 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> >
> > Amit Kapila <amit.kapil...@gmail.com> writes:
> > > On Tue, Aug 17, 2021 at 6:40 AM Peter Smith <smithpb2...@gmail.com> wrote:
> > >> On Mon, Aug 16, 2021 at 11:31 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> > >>> Abstractly it'd be
> > >>>
> > >>> createpub := CREATE PUBLICATION pubname FOR cpitem [, ... ] [ WITH ... ]
> > >>>
> > >>> cpitem := ALL TABLES |
> > >>> TABLE name |
> > >>> ALL TABLES IN SCHEMA name |
> > >>> ALL SEQUENCES |
> > >>> SEQUENCE name |
> > >>> ALL SEQUENCES IN SCHEMA name |
> > >>> name
> > >>>
> > >>> The grammar output would need some post-analysis to attribute the
> > >>> right type to bare "name" items, but that doesn't seem difficult.
> >
> > >> That last bare "name" cpitem. looks like it would permit the following
> > >> syntax:
> > >> CREATE PUBLICATION pub FOR a,b,c;
> > >> Was that intentional?
> >
> > > I think so.
> >
> > I had supposed that we could throw an error at the post-processing stage,
> > or alternatively default to assuming that such names are tables.
> >
> > Now you could instead make the grammar work like
> >
> > cpitem := ALL TABLES |
> > TABLE name [, ...] |
> > ALL TABLES IN SCHEMA name [, ...] |
> > ALL SEQUENCES |
> > SEQUENCE name [, ...] |
> > ALL SEQUENCES IN SCHEMA name [, ...]
> >
> > which would result in a two-level-list data structure. I'm not sure
> > that this is better, as any sort of mistake would result in a very
> > uninformative generic "syntax error" from Bison. Errors out of a
> > post-processing stage could be more specific than that.
>
> I preferred the implementation in the lines Tom Lane had proposed at [1]. Is
> it ok if the implementation is something like below:
> CreatePublicationStmt:
> CREATE PUBLICATION name FOR pub_obj_list opt_definition
> {
> CreatePublicationStmt *n = makeNode(CreatePublicationStmt);
> n->pubname = $3;
> n->options = $6;
> n->pubobjects = (List *)$5;
> $$ = (Node *)n;
> }
> ;
> pub_obj_list: PublicationObjSpec
> { $$ = list_make1($1); }
> | pub_obj_list ',' PublicationObjSpec
> { $$ = lappend($1, $3); }
> ;
> /* FOR TABLE and FOR ALL TABLES IN SCHEMA specifications */
> PublicationObjSpec: TABLE pubobj_expr
> { ....}
> | ALL TABLES IN_P SCHEMA pubobj_expr
> { ....}
> | pubobj_expr
> { ....}
> ;
> pubobj_expr:
> any_name
> { ....}
> | any_name '*'
> { ....}
> | ONLY any_name
> { ....}
> | ONLY '(' any_name ')'
> { ....}
> | CURRENT_SCHEMA
> { ....}
> ;
"FOR ALL TABLES” (that includes all tables in the database) is missing
in this syntax?
>
> I needed pubobj_expr to support the existing syntaxes supported by
> relation_expr and also to handle CURRENT_SCHEMA support in case of the "FOR
> ALL TABLES IN SCHEMA" feature. I changed the name to any_name to support
> objects like "sch1.t1".
I think that relation_expr also accepts objects like "sch1.t1", no?
> I felt if a user specified "FOR ALL TABLES", the user should not be allowed
> to combine it with "FOR TABLE" and "FOR ALL TABLES IN SCHEMA" as "FOR ALL
> TABLES" anyway will include all the tables.
I think so too.
> Should we support the similar syntax in case of alter publication, like
> "ALTER PUBLICATION pub1 ADD TABLE t1,t2, ALL TABLES IN SCHEMA sch1, sch2" or
> shall we keep these separate like "ALTER PUBLICATION pub1 ADD TABLE t1, t2"
> and "ALTER PUBLICATION pub1 ADD ALL TABLES IN SCHEMA sch1, sch2". I preferred
> to keep it separate as we have kept ADD/DROP separately which cannot be
> combined currently.
If we support the former syntax, the latter two syntaxes are also
supported. Why do we want to support only the latter separate two
syntaxes?
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
