On Fri, Nov 5, 2021 at 10:44 AM Peter Smith <smithpb2...@gmail.com> wrote: > > PSA new set of v37* patches. >
Few comments about changes made to the patch to rebase it: 1. +#if 1 + // FIXME - can we do a better job if integrating this with the schema changes + /* + * Remove all publication-table mappings. We could possibly remove (i) + * tables that are not found in the new table list and (ii) tables that + * are being re-added with a different qual expression. For (ii), + * simply updating the existing tuple is not enough, because of qual + * expression dependencies. + */ + foreach(oldlc, oldrelids) + { + Oid oldrelid = lfirst_oid(oldlc); + PublicationRelInfo *oldrel; + + oldrel = palloc(sizeof(PublicationRelInfo)); + oldrel->relid = oldrelid; + oldrel->whereClause = NULL; + oldrel->relation = table_open(oldrel->relid, + ShareUpdateExclusiveLock); + delrels = lappend(delrels, oldrel); + } +#else CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist, PUBLICATIONOBJ_TABLE); I think for the correct merge you need to just call CheckObjSchemaNotAlreadyInPublication() before this for loop. BTW, I have a question regarding this implementation. Here, it has been assumed that the new rel will always be specified with a different qual, what if there is no qual or if the qual is the same? 2. +preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner, bool alter_drop) { ListCell *cell; PublicationObjSpec *pubobj; @@ -17341,7 +17359,15 @@ preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner) errcode(ERRCODE_SYNTAX_ERROR), errmsg("invalid table name at or near"), parser_errposition(pubobj->location)); - else if (pubobj->name) + + /* cannot use WHERE w-filter for DROP TABLE from publications */ + if (pubobj->pubtable && pubobj->pubtable->whereClause && alter_drop) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("invalid use of WHERE row-filter in ALTER PUBLICATION ... DROP TABLE"), + parser_errposition(pubobj->location)); + This change looks a bit ad-hoc to me. Can we handle this at a later point of time in publicationcmds.c? 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. -- With Regards, Amit Kapila.