Re: Skipping schema changes in publication

On Mon, Mar 9, 2026 at 4:23 PM vignesh C <vignesh21@gmail.com> wrote:
>
> This is addressed in the v60 version patch attached.
> Also Nisha's comments from [1] and Shveta's comments from [2] are
> handled in the v60 version.
>

*
+ /* Check that user is allowed to manipulate the publication. */
+ if (excepttables && !pubform->puballtables)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is not defined as FOR ALL TABLES",
+    NameStr(pubform->pubname)),
+ errdetail("Except tables cannot be added to or dropped from
publications that are not defined as FOR ALL TABLES."));
+
+ /*
+ * SET ALL TABLES is allowed only for publications defined as FOR ALL
+ * TABLES.
+ */
+ if (stmt->action == AP_SetObjects && !pubform->puballtables &&
+ !tables && !excepttables && !schemaidlist)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is not defined as FOR ALL TABLES",
+    NameStr(pubform->pubname)));
+
+ /* Only a superuser can add or remove EXCEPT tables */
+ if (stmt->action == AP_SetObjects &&
+ (excepttables || (!tables && !excepttables && !schemaidlist)) &&
+ !superuser())
+ ereport(ERROR,
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to add or drop except tables"));

All the above checks are trying to drive the answer to whether the new
Alter statement is for_all_tables via indirect checks. Why not keep
for_all_tables boolean variable in AlterPublicationStmt similar to
CreatePublicationStmt? That should simplify the above checks.

* Also, the superuser check should be earlier in the code, say just
after following existing check:
if ((stmt->action == AP_AddObjects || stmt->action == AP_SetObjects) &&
schemaidlist && !superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to add or set schemas")));

-- 
With Regards,
Amit Kapila.