On Thu, 5 Mar 2026 at 17:22, shveta malik <[email protected]> wrote: > > On Thu, Mar 5, 2026 at 3:05 PM vignesh C <[email protected]> wrote: > > > > > > Here is the rebased patch for the remaining items in the patch series. > > > > Thanks. Please find a few comments for 001: > > > 1) > alter_publication.sgml: > > The SET clause will replace the list of except tables/tables/schemas > in the publication with the specified list > > The order is difficult to read. Would it be better to say it in this > order: tables/schemas/except-tables
I have changed it to tables/except tables/schemas to keep tables together
> 2)
> postgres=# ALTER TABLE tab_top_root ATTACH PARTITION tab_root FOR
> VALUES FROM (0) TO (2000);
> ERROR: cannot attach table "tab_root" as partition because it is
> referenced in publications "pub1", "pub3" EXCEPT clause
> DETAIL: The publication EXCEPT clause cannot contain tables that are
> partitions.
> HINT: Modify the publication's EXCEPT clause using ALTER PUBLICATION
> ... SET EXCEPT TABLE or DROP EXCEPT TABLE before attaching the table.
>
> Shall we shorten the HINT slightly? Is it necessary to explicitly say
> "before attaching the table", or can we omit it? Thoughts?
Modified to omit it.
> 3)
> + if (has_except_table)
> + ereport(ERROR,
> + errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("EXCEPT TABLE clause allowed only for ALL TABLES PUBLICATION"));
> +
>
> In what scenario will we hit it? I could not find one.
It will hit in this scenario, also added a test for thi:
create publication pub2 for except table (t1);
ERROR: EXCEPT TABLE clause allowed only for ALL TABLES PUBLICATION
> 4)
> -- fail - can't add an EXCEPT TABLE to 'FOR TABLE' publication
> ALTER PUBLICATION testpub_fortable ADD EXCEPT TABLE (testpub_tbl1);
> ERROR: EXCEPT TABLE clause allowed only for SET clause
>
> Even though the comment says that it is a test for not allowing EXCEPT
> TABLE for 'FOR TABLE' (perhaps trying to hit the error in comment 3),
> it ultimately hits another error that EXCEPT is not supported with
> 'ADD'. I think the error in comment-3 is unreachable. Or let me know
> if otherwise.
> We need to correct the comment here.
This test can be removed as the error comes from the parser itself and
there is one test already with testpub_foralltables_excepttable. This
will be returned from:
| ALTER PUBLICATION name ADD_P pub_obj_list
{
+ bool has_except_table = false;
AlterPublicationStmt *n =
makeNode(AlterPublicationStmt);
n->pubname = $3;
n->pubobjects = $5;
-
preprocess_pubobj_list(n->pubobjects, yyscanner);
+ has_except_table =
preprocess_pubobj_list(n->pubobjects,
+
yyscanner);
+ if (has_except_table)
+ ereport(ERROR,
+
errcode(ERRCODE_SYNTAX_ERROR),
+
errmsg("EXCEPT TABLE clause allowed only for SET clause"));
+
n->action = AP_AddObjects;
$$ = (Node *) n;
}
> 5)
> -- fail - can't drop an EXCEPT TABLE from 'FOR TABLE' publication
> ALTER PUBLICATION testpub_fortable DROP EXCEPT TABLE (testpub_tbl1);
> ERROR: EXCEPT TABLE clause allowed only for SET clause
>
> Same is valid for this, comment (i.e. the intent) and error does not match.
This test can be removed as the error comes from the parser itself and
there is one test already with testpub_foralltables_excepttable.
Thanks for the comments, the attached v58 version patch has the
changes for the same.
Regards,
Vignesh
v58-0002-Support-DROP-EXCEPT-TABLE-in-ALTER-PUBLICATION.patch
Description: Binary data
v58-0001-Support-SET-EXCEPT-TABLE-in-ALTER-PUBLICATION.patch
Description: Binary data
