Improve documentation of publication privilege checks

Started by Shlok Kyal20 days ago8 messages
#1Shlok Kyal
shlok.kyal.oss@gmail.com
1 attachment(s)

Hi Hackers,

While reviewing the Security section of the logical replication
documentation, I felt that the description of privilege requirements
for publications is ambiguous, and clarity could be improved by
explicitly mentioning the associated SQL syntax. Thoughts?

Thanks,
Shlok Kyal

Attachments:

v1-0001-Improve-documentation-of-publication-privilege-ch.patchapplication/octet-stream; name=v1-0001-Improve-documentation-of-publication-privilege-ch.patchDownload
From 4a652cbfd2e490d89594375b042b5d8719933027 Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Tue, 23 Dec 2025 14:22:06 +0530
Subject: [PATCH v1] Improve documentation of publication privilege checks

Make the logical replication documentation explicitly describe the
privilege requirements for different publication syntaxes.
---
 doc/src/sgml/logical-replication.sgml | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index b3faaa675ef..7d8413d7c6b 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2550,10 +2550,12 @@ CONTEXT:  processing remote data for replication origin "pg_16395" during "INSER
   </para>
 
   <para>
-   To add tables to a publication, the user must have ownership rights on the
-   table. To add all tables in schema to a publication, the user must be a
-   superuser. To create a publication that publishes all tables, all tables in
-   schema, or all sequences automatically, the user must be a superuser.
+   To create a publication using <literal>FOR ALL TABLES</literal>,
+   <literal>FOR ALL SEQUENCES</literal> or
+   <literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser. To add
+   <literal>TABLES IN SCHEMA</literal> to a publication, the user must be a
+   superuser. To add tables to a publication, the user must have ownership
+   rights on the table.
   </para>
 
   <para>
-- 
2.34.1

#2Chao Li
li.evan.chao@gmail.com
In reply to: Shlok Kyal (#1)
Re: Improve documentation of publication privilege checks

On Dec 23, 2025, at 16:59, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:

Hi Hackers,

While reviewing the Security section of the logical replication
documentation, I felt that the description of privilege requirements
for publications is ambiguous, and clarity could be improved by
explicitly mentioning the associated SQL syntax. Thoughts?

Thanks,
Shlok Kyal
<v1-0001-Improve-documentation-of-publication-privilege-ch.patch>

I have no objection to this patch. Just the new phrase sounds a little redundant as “FOR TABLES IN SCHEMA” is mentioned twice back-to-back. I tried to rephrase like:

```
To create a publication that automatically publishes objects using
<literal>FOR ALL TABLES</literal>,
<literal>FOR ALL SEQUENCES</literal>, or
<literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser.
Likewise, adding tables using <literal>TABLES IN SCHEMA</literal> with
<command>ALTER PUBLICATION</command> requires superuser privileges.
To add individual tables to a publication, the user must have ownership rights on the table.
```

I am open if you accept my suggestion or try to enhance the phrase on your own.

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

#3Peter Smith
smithpb2250@gmail.com
In reply to: Chao Li (#2)
1 attachment(s)
Re: Improve documentation of publication privilege checks

Hi Shlok/Chao-San.

How about this alternative wording:

<para>
To create a publication using any of <literal>FOR ALL TABLES</literal>,
<literal>FOR ALL SEQUENCES</literal>, or
<literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser.
To alter a publication using <literal>ADD TABLE</literal>, the user must
have ownership rights on the table. To alter a publication using
<literal>ADD TABLES IN SCHEMA</literal>, the user must be a superuser.
</para>

IMO this is both simpler and more consistent. PSA a diff for the same.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

PS_wording.diffapplication/octet-stream; name=PS_wording.diffDownload
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index b3faaa6..7ca2ccb 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2550,10 +2550,12 @@ CONTEXT:  processing remote data for replication origin "pg_16395" during "INSER
   </para>
 
   <para>
-   To add tables to a publication, the user must have ownership rights on the
-   table. To add all tables in schema to a publication, the user must be a
-   superuser. To create a publication that publishes all tables, all tables in
-   schema, or all sequences automatically, the user must be a superuser.
+   To create a publication using any of <literal>FOR ALL TABLES</literal>,
+   <literal>FOR ALL SEQUENCES</literal>, or
+   <literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser.
+   To alter a publication using <literal>ADD TABLE</literal>, the user must
+   have ownership rights on the table.  To alter a publication using
+   <literal>ADD TABLES IN SCHEMA</literal>, the user must be a superuser.
   </para>
 
   <para>
#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Peter Smith (#3)
Re: Improve documentation of publication privilege checks

On Tue, Dec 23, 2025 at 4:43 PM Peter Smith <smithpb2250@gmail.com> wrote:

Hi Shlok/Chao-San.

How about this alternative wording:

<para>
To create a publication using any of <literal>FOR ALL TABLES</literal>,
<literal>FOR ALL SEQUENCES</literal>, or
<literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser.
To alter a publication using <literal>ADD TABLE</literal>, the user must
have ownership rights on the table. To alter a publication using
<literal>ADD TABLES IN SCHEMA</literal>, the user must be a superuser.
</para>

I initially preferred Chao Li's version but upon deeper consideration I've
settled on this variant. The conjunctions in the other are nice, but I've
come to like how create and alter are better separated here. And the
choice to list "add table" first breaks up the string of superuser required
commands when switching from creating to altering.

Kinda feel we should start this with the individual table creation case
though:

To create a publication using FOR TABLE, the user must have ownership
rights on all listed tables. To create a publication using any of ... the
user must be a superuser. To alter ...

The alter case likewise accepts multiple tables...

David J.

#5Shlok Kyal
shlok.kyal.oss@gmail.com
In reply to: David G. Johnston (#4)
1 attachment(s)
Re: Improve documentation of publication privilege checks

On Wed, 24 Dec 2025 at 06:23, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Tue, Dec 23, 2025 at 4:43 PM Peter Smith <smithpb2250@gmail.com> wrote:

Hi Shlok/Chao-San.

How about this alternative wording:

<para>
To create a publication using any of <literal>FOR ALL TABLES</literal>,
<literal>FOR ALL SEQUENCES</literal>, or
<literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser.
To alter a publication using <literal>ADD TABLE</literal>, the user must
have ownership rights on the table. To alter a publication using
<literal>ADD TABLES IN SCHEMA</literal>, the user must be a superuser.
</para>

I initially preferred Chao Li's version but upon deeper consideration I've settled on this variant. The conjunctions in the other are nice, but I've come to like how create and alter are better separated here. And the choice to list "add table" first breaks up the string of superuser required commands when switching from creating to altering.

Kinda feel we should start this with the individual table creation case though:

To create a publication using FOR TABLE, the user must have ownership rights on all listed tables. To create a publication using any of ... the user must be a superuser. To alter ...

The alter case likewise accepts multiple tables...

Thanks Chao-san, Peter and David for reviewing the patch. I also felt
the version shared by Peter is more appropriate. I have made the
suggested changes by David.

Thanks,
Shlok Kyal

Attachments:

v2-0001-Improve-documentation-of-publication-privilege-ch.patchapplication/octet-stream; name=v2-0001-Improve-documentation-of-publication-privilege-ch.patchDownload
From 0fe29373dfd8bdda1674fad1b4ddb3de45dbe195 Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Tue, 23 Dec 2025 14:22:06 +0530
Subject: [PATCH v2] Improve documentation of publication privilege checks

Make the logical replication documentation explicitly describe the
privilege requirements for different publication syntaxes.
---
 doc/src/sgml/logical-replication.sgml | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index f47b7378397..58ce75d8b63 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2550,10 +2550,14 @@ CONTEXT:  processing remote data for replication origin "pg_16395" during "INSER
   </para>
 
   <para>
-   To add tables to a publication, the user must have ownership rights on the
-   table. To add all tables in schema to a publication, the user must be a
-   superuser. To create a publication that publishes all tables, all tables in
-   schema, or all sequences automatically, the user must be a superuser.
+   To create a publication using <literal>FOR TABLE</literal>, the user must
+   have ownership rights on all the listed tables. To create a publication
+   using any of <literal>FOR ALL TABLES</literal>,
+   <literal>FOR ALL SEQUENCES</literal>,
+   or <literal>FOR TABLES IN SCHEMA</literal>, the user must be a superuser. To
+   alter a publication using <literal>ADD TABLE</literal>, the user must have
+   ownership rights on all the listed tables. To alter a publication using
+   <literal>ADD TABLES IN SCHEMA</literal>, the user must be a superuser.
   </para>
 
   <para>
-- 
2.34.1

#6Peter Smith
smithpb2250@gmail.com
In reply to: Shlok Kyal (#5)
Re: Improve documentation of publication privilege checks

Thanks Chao-san, Peter and David for reviewing the patch. I also felt
the version shared by Peter is more appropriate. I have made the
suggested changes by David.

Patch v2 LGTM.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

#7Chao Li
li.evan.chao@gmail.com
In reply to: Peter Smith (#6)
Re: Improve documentation of publication privilege checks

On Dec 24, 2025, at 14:57, Peter Smith <smithpb2250@gmail.com> wrote:

Thanks Chao-san, Peter and David for reviewing the patch. I also felt
the version shared by Peter is more appropriate. I have made the
suggested changes by David.

Patch v2 LGTM.

+1

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

#8David G. Johnston
david.g.johnston@gmail.com
In reply to: Chao Li (#7)
Re: Improve documentation of publication privilege checks

On Wed, Dec 24, 2025 at 12:03 AM Chao Li <li.evan.chao@gmail.com> wrote:

On Dec 24, 2025, at 14:57, Peter Smith <smithpb2250@gmail.com> wrote:

Thanks Chao-san, Peter and David for reviewing the patch. I also felt
the version shared by Peter is more appropriate. I have made the
suggested changes by David.

Patch v2 LGTM.

+1

WFM

David J.