| From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: [19] CREATE SUBSCRIPTION ... SERVER |
| Date: | 2026-03-05 08:52:57 |
| Message-ID: | 80303af653a3da2a94e32ff91d6538675e921bf0.camel@j-davis.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, 2026-03-05 at 09:21 +0530, Amit Kapila wrote:
> We revoke view rights on subconninfo from the public. See below [A]
> in
> system_views.sql. Do we want to do the same for subserver or is it
> okay for users to see it?
I can't think of a reason that the server name should be secret, but
let me know if you think so.
> I think the following comment and some place
> in docs needs to be updated.
> [A]
> -- All columns of pg_subscription except subconninfo are publicly
> readable.
> REVOKE ALL ON pg_subscription FROM public;
> GRANT SELECT (oid, subdbid, subskiplsn, subname, subowner,
Good catch! Thank you.
> 2. We may want to update the following text in pg_dump docs about the
> new way of connecting to hosts. See [B] (When dumping logical
> replication subscriptions, pg_dump will generate CREATE SUBSCRIPTION
> commands that use the connect = false option, so that restoring the
> subscription does not make remote connections for creating a
> replication slot or for initial table copy. That way, the dump can be
> restored without requiring network access to the remote servers. It
> is
> then up to the user to reactivate the subscriptions in a suitable
> way.
> If the involved hosts have changed, the connection information might
> have to be changed.)
>
> [B] - https://www.postgresql.org/docs/devel/app-pgdump.html
>
I think the above comment is still correct -- it would be a bit easier
to deal with servers rather than raw connection strings, but the
comment already says "...might have to be changed" which is just a
reminder to look.
Attached a new patch that also addressed the review comments from here:
Additionally, I ran into a problem that's worth highlighting:
DROP SERVER ... CASCADE was broken, because the subscription is
dependent on it but that's in a global catalog, which is not handled by
doDeletion(). The subscription is conceptually a per-database object,
but it's in a shared catalog with a subdbid field. I solved that
problem by adding a guard to findDependentObjects() to check for the
referenced object belonging to a shared catalog, and if so it just
throws an error (so CASCADE is not supported for servers used in
subscriptions). That's a simple but not a very satisfying solution, so
let me know if you see a problem with that.
Regards,
Jeff Davis
| Attachment | Content-Type | Size |
|---|---|---|
| v19-0001-CREATE-SUBSCRIPTION-.-SERVER.patch | text/x-patch | 131.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Antonin Houska | 2026-03-05 09:29:41 | Dead code in logical decoding of speculative insertions |
| Previous Message | Chao Li | 2026-03-05 08:52:45 | Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access) |