| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Distinguish publication exclusions in object addresses |
| Date: | 2026-09-16 04:50:32 |
| Message-ID: | CAHut+PvQgoi2G6QHKG_R-eM-VG6GD-PXWnCrGNS_YEoMaPxpWQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Some review comments for v3:
======
Commit Message
1.
Fix this by distinguishing EXCEPT entries in the object address code.
They are now reported as publication excluded relation, with object
identities indicating that the table is excluded from the publication.
~
Should "publication excluded relation" be quoted here?
======
src/backend/catalog/aclchk.c
+ case OBJECT_PUBLICATION_EXCLUDED_REL:
case OBJECT_PUBLICATION_NAMESPACE:
case OBJECT_PUBLICATION_REL:
The new enum name OBJECT_PUBLICATION_EXCLUDED_REL is closely related
to OBJECT_PUBLICATION_REL.
IMO, a better name would be OBJECT_PUBLICATION_REL_EXCLUDED, so these
related things are kept adjacent alphabetically and in the code.
(same comment affects multiple other files but not repeating all those
in this post)
======
src/backend/catalog/objectaddress.c
2.
static ObjectAddress
-get_object_address_publication_rel(List *object,
+get_object_address_publication_rel(ObjectType objtype, List *object,
Relation *relp, bool missing_ok)
If not going to describe parameter `objType` then maybe a
self-documenting Assert would be good to have here.
~~~
3.
+ /*
+ * The same relation and publication pair identifies either a published or
+ * an excluded relation, so reject an entry of the kind that was not asked
+ * for.
+ */
The wording looks a bit strange. Particularly the 2nd part ("reject an
entry of the kind that was not asked for")
SUGGESTION:
A given relation/publication pair can represent either a published
relation or an excluded one, but not both. Reject the entry if it is
not the kind the caller asked for.
~~~
4.
+ if (isexcept)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a published relation of publication \"%s\"",
+ RelationGetRelationName(relation), pubname)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not an excluded relation of publication \"%s\"",
+ RelationGetRelationName(relation), pubname)));
Publications can have the same table name in multiple schemas. The
name should be fully-qualified in the errmsg to eliminate any
ambiguity.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-09-16 04:56:34 | Re: remove_useless_joins vs. bug #19560 |
| Previous Message | Nisha Moond | 2026-09-16 04:48:51 | Re: Introduce XID age based replication slot invalidation |