comment/security label for publication/subscription

On Fri, Mar 24, 2017 at 12:18 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

Here is a patch to add COMMENT support for publications and subscriptions.

On a similar issue, do we need SECURITY LABEL support for those? Does
that make sense?

IMHO, it's good to have COMMENT and SECURITY LABEL support for pretty
much everything.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Robert Haas (robertmhaas@gmail.com) wrote:

On Fri, Mar 24, 2017 at 12:18 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

Here is a patch to add COMMENT support for publications and subscriptions.

On a similar issue, do we need SECURITY LABEL support for those? Does
that make sense?

IMHO, it's good to have COMMENT and SECURITY LABEL support for pretty
much everything.

+1

Thanks!

Stephen

Hi,

On 2017-03-24 00:18:26 -0400, Peter Eisentraut wrote:

Here is a patch to add COMMENT support for publications and subscriptions.

On a similar issue, do we need SECURITY LABEL support for those? Does
that make sense?

It looks like this was committed (87dee41f3ed).

Unfortunately I found, during an investigation of something completely
independent, that it leads to comments and (and presumably security labels) to
be orphaned on DROP.

In fact, our regression database actually contains such an orphaned comment:

regression[1536656][1]=# SELECT * FROM pg_description WHERE classoid = 'pg_subscription'::regclass;
┌────────┬──────────┬──────────┬───────────────────┐
│ objoid │ classoid │ objsubid │ description │
├────────┼──────────┼──────────┼───────────────────┤
│ 123718 │ 6100 │ 0 │ test subscription │
└────────┴──────────┴──────────┴───────────────────┘
(1 row)

Seems we need to beef up oidjoins.sql to find orphaned objects.

I can't entirely blame this commit, it seems pretty cruddy that the drop
routine of every global object needs to have a synchronized copy of various
Delete* routines. It's bad enough that drop functions for global objects need
to know about having to drop dependencies manually, but copying the set of
objects that need to be dropped in each seems like a bad idea.

Trivial repro:

DROP SUBSCRIPTION IF EXISTS s;

CREATE SUBSCRIPTION s CONNECTION '' PUBLICATION p
WITH (connect = false, slot_name = NONE);
COMMENT ON SUBSCRIPTION s IS 'leaked';
DROP SUBSCRIPTION s;

SELECT * FROM pg_description WHERE classoid = 'pg_subscription'::regclass;

Which will show something like:
┌────────┬──────────┬──────────┬─────────────┐
│ objoid │ classoid │ objsubid │ description │
├────────┼──────────┼──────────┼─────────────┤
│ 116868 │ 6100 │ 0 │ leaked │
└────────┴──────────┴──────────┴─────────────┘

Greetings,

Andres Freund

On Aug 9, 2026, at 01:35, Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2017-03-24 00:18:26 -0400, Peter Eisentraut wrote:

Here is a patch to add COMMENT support for publications and subscriptions.

On a similar issue, do we need SECURITY LABEL support for those? Does
that make sense?

It looks like this was committed (87dee41f3ed).

Unfortunately I found, during an investigation of something completely
independent, that it leads to comments and (and presumably security labels) to
be orphaned on DROP.

In fact, our regression database actually contains such an orphaned comment:

regression[1536656][1]=# SELECT * FROM pg_description WHERE classoid = 'pg_subscription'::regclass;
┌────────┬──────────┬──────────┬───────────────────┐
│ objoid │ classoid │ objsubid │ description │
├────────┼──────────┼──────────┼───────────────────┤
│ 123718 │ 6100 │ 0 │ test subscription │
└────────┴──────────┴──────────┴───────────────────┘
(1 row)

Seems we need to beef up oidjoins.sql to find orphaned objects.

I can't entirely blame this commit, it seems pretty cruddy that the drop
routine of every global object needs to have a synchronized copy of various
Delete* routines. It's bad enough that drop functions for global objects need
to know about having to drop dependencies manually, but copying the set of
objects that need to be dropped in each seems like a bad idea.

Trivial repro:

DROP SUBSCRIPTION IF EXISTS s;

CREATE SUBSCRIPTION s CONNECTION '' PUBLICATION p
WITH (connect = false, slot_name = NONE);
COMMENT ON SUBSCRIPTION s IS 'leaked';
DROP SUBSCRIPTION s;

SELECT * FROM pg_description WHERE classoid = 'pg_subscription'::regclass;

Which will show something like:
┌────────┬──────────┬──────────┬─────────────┐
│ objoid │ classoid │ objsubid │ description │
├────────┼──────────┼──────────┼─────────────┤
│ 116868 │ 6100 │ 0 │ leaked │
└────────┴──────────┴──────────┴─────────────┘

Greetings,

Andres Freund

I just debugged the code. Dropping a publication uses the generic deleteOneObject() path, so that comment, security label etc. dependencies are deleted automatically. While for some reason, doDeletion() explicitly reject subscription:
```
/*
* These global object types are not supported here.
*/
case AuthIdRelationId:
case DatabaseRelationId:
case TableSpaceRelationId:
case SubscriptionRelationId:
case ParameterAclRelationId:
elog(ERROR, "global objects cannot be deleted by doDeletion");
break;
```

Therefore, DropSubscription() has to perform the cleanup explicitly. I guess that is why commit 87dee41f3ed missed adding the deletion of comments and security labels to DropSubscription().

I have prepared a patch to fix the bug. The fix is straightforward, and I have added tests for both comments and security labels.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/