Обсуждение: Documentation improvement patch

Dear PostgreSQL Community,

This is a kind reminder regarding my documentation patch submitted a month ago.

I am still very interested in contributing these improvements and would be grateful for a review when time permits.

The patch can be also applied to the master branch.

Thank you for your consideration.

--
Regards,
Oleg Sibiryakov
Technical Writer
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

Dear all,

I have prepared a patch containing some minor inconsistencies in the documentation. Please, take a look.

I will be looking forward to your feedback.

The patch shall be applied to the REL_18_STABLE branch.

--
Regards,
Oleg Sibiryakov
Technical Writer
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

> On 10 Sep 2025, at 09:54, Oleg <o.sibiryakov@postgrespro.ru> wrote:
>
> Dear all,
> I have prepared a patch containing some minor inconsistencies in the documentation. Please, take a look.
> I will be looking forward to your feedback.

Thanks for the patch, while most of these are obvious improvements I have a few
comments on some:


-       Change the definition of a replication slot.
+       Changes the definition of a replication slot.
Reading this page it seems we are mixing tense in many places, some say "Change
the" and "Read some" and elsewhere we use "Drops the".  Maybe a more holistic
approach would be better for this page to improve consistency?


-       Not enabled by default because it is resource intensive.
+       Not enabled by default because it is resource-intensive.
We use both spellings in multiple places, shouldn't all be changed?


-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.
        ...
-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.
        ...
-       <command>COPY</command> and other functions which allow executing a
+       the <command>COPY</command> command and functions, which allow executing a
I'm not sure about these, I think we use COPY without the the "the COPY
command" decoration in many places so I think it's more consistent like this.


-     to call functions defined in the standard internal library, by using an
+     to call functions defined in the standard internal function library by using an
      interface similar to their SQL signature.
Isn't it a bit redundant to say "internal function library" when we are already
talking about function definitions?

> The patch shall be applied to the REL_18_STABLE branch.

As you mentioned downthread, this is also for master.  Our workflow is to
always apply to master and backpatch from there.

--
Daniel Gustafsson

Thank you for your feedback, Daniel.

My thoughts are below:

-       Change the definition of a replication slot.
+       Changes the definition of a replication slot.
Reading this page it seems we are mixing tense in many places, some say "Change
the" and "Read some" and elsewhere we use "Drops the".  Maybe a more holistic
approach would be better for this page to improve consistency?

I agree, let's add "s" in all cases for the sake of consistency.

-       Not enabled by default because it is resource intensive.
+       Not enabled by default because it is resource-intensive.
We use both spellings in multiple places, shouldn't all be changed?

Agreed, changing all instances to "resource-intensive".

-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other functions which allow executing a
+       the <command>COPY</command> command and functions, which allow executing a
I'm not sure about these, I think we use COPY without the the "the COPY
command" decoration in many places so I think it's more consistent like this.

I actually think we should add the decoration here because "<command>COPY</command> and other file-access functions"
sounds a bit confusing since COPY is not a file-access function and we seem to put it in the list. Even though I 
agree that everybody knows COPY is a command, not a function.


-     to call functions defined in the standard internal library, by using an
+     to call functions defined in the standard internal function library by using an      interface similar to their SQL signature.
Isn't it a bit redundant to say "internal function library" when we are already
talking about function definitions?

I agree that it may seem redundant, I added "function" here for the sake of consistency with lines 1829/1830 (if applied to the master branch)
where the documentation mentions "standard internal function library".

Please, let me know what you think of the last two points for me to send the updated patch.

--
Oleg Sibiryakov

On 10 Sep 2025, at 09:54, Oleg <o.sibiryakov@postgrespro.ru> wrote:

Dear all,
I have prepared a patch containing some minor inconsistencies in the documentation. Please, take a look.
I will be looking forward to your feedback.

Thanks for the patch, while most of these are obvious improvements I have a few
comments on some:


-       Change the definition of a replication slot.
+       Changes the definition of a replication slot.
Reading this page it seems we are mixing tense in many places, some say "Change
the" and "Read some" and elsewhere we use "Drops the".  Maybe a more holistic
approach would be better for this page to improve consistency?


-       Not enabled by default because it is resource intensive.
+       Not enabled by default because it is resource-intensive.
We use both spellings in multiple places, shouldn't all be changed?


-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other functions which allow executing a
+       the <command>COPY</command> command and functions, which allow executing a
I'm not sure about these, I think we use COPY without the the "the COPY
command" decoration in many places so I think it's more consistent like this.


-     to call functions defined in the standard internal library, by using an
+     to call functions defined in the standard internal function library by using an      interface similar to their SQL signature.
Isn't it a bit redundant to say "internal function library" when we are already
talking about function definitions?

The patch shall be applied to the REL_18_STABLE branch.

As you mentioned downthread, this is also for master.  Our workflow is to
always apply to master and backpatch from there.

--
Daniel Gustafsson

Dear Daniel,

Could you please provide your feedback on the last two points?
Once I have it, I will send the updated patch immediately to finalize the improvements.

Thank you,
Oleg

Thank you for your feedback, Daniel.

My thoughts are below:

-       Change the definition of a replication slot.
+       Changes the definition of a replication slot.
Reading this page it seems we are mixing tense in many places, some say "Change
the" and "Read some" and elsewhere we use "Drops the".  Maybe a more holistic
approach would be better for this page to improve consistency?

I agree, let's add "s" in all cases for the sake of consistency.

-       Not enabled by default because it is resource intensive.
+       Not enabled by default because it is resource-intensive.
We use both spellings in multiple places, shouldn't all be changed?

Agreed, changing all instances to "resource-intensive".

-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other functions which allow executing a
+       the <command>COPY</command> command and functions, which allow executing a
I'm not sure about these, I think we use COPY without the the "the COPY
command" decoration in many places so I think it's more consistent like this.

I actually think we should add the decoration here because "<command>COPY</command> and other file-access functions"
sounds a bit confusing since COPY is not a file-access function and we seem to put it in the list. Even though I 
agree that everybody knows COPY is a command, not a function.


-     to call functions defined in the standard internal library, by using an
+     to call functions defined in the standard internal function library by using an      interface similar to their SQL signature.
Isn't it a bit redundant to say "internal function library" when we are already
talking about function definitions?

I agree that it may seem redundant, I added "function" here for the sake of consistency with lines 1829/1830 (if applied to the master branch)
where the documentation mentions "standard internal function library".

Please, let me know what you think of the last two points for me to send the updated patch.

--
Oleg Sibiryakov

On 10.10.2025 10:15, Daniel Gustafsson wrote:

On 10 Sep 2025, at 09:54, Oleg <o.sibiryakov@postgrespro.ru> wrote:

Dear all,
I have prepared a patch containing some minor inconsistencies in the documentation. Please, take a look.
I will be looking forward to your feedback.

Thanks for the patch, while most of these are obvious improvements I have a few
comments on some:


-       Change the definition of a replication slot.
+       Changes the definition of a replication slot.
Reading this page it seems we are mixing tense in many places, some say "Change
the" and "Read some" and elsewhere we use "Drops the".  Maybe a more holistic
approach would be better for this page to improve consistency?


-       Not enabled by default because it is resource intensive.
+       Not enabled by default because it is resource-intensive.
We use both spellings in multiple places, shouldn't all be changed?


-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other file-access functions.
+       the <command>COPY</command> command and file-access functions.        ...
-       <command>COPY</command> and other functions which allow executing a
+       the <command>COPY</command> command and functions, which allow executing a
I'm not sure about these, I think we use COPY without the the "the COPY
command" decoration in many places so I think it's more consistent like this.


-     to call functions defined in the standard internal library, by using an
+     to call functions defined in the standard internal function library by using an      interface similar to their SQL signature.
Isn't it a bit redundant to say "internal function library" when we are already
talking about function definitions?

The patch shall be applied to the REL_18_STABLE branch.

As you mentioned downthread, this is also for master.  Our workflow is to
always apply to master and backpatch from there.

--
Daniel Gustafsson

> On 13 Oct 2025, at 12:51, Oleg <o.sibiryakov@postgrespro.ru> wrote:

> - <command>COPY</command> and other functions which allow executing a
> + the <command>COPY</command> command and functions, which allow executing a
> I'm not sure about these, I think we use COPY without the the "the COPY
> command" decoration in many places so I think it's more consistent like this.
>
> I actually think we should add the decoration here because "<command>COPY</command> and other file-access functions"
> sounds a bit confusing since COPY is not a file-access function and we seem to put it in the list. Even though I
> agree that everybody knows COPY is a command, not a function.

We refer to SQL commands by just their names all over the documentation without
saying "an EXPLAIN command" etc, and I think this falls in that same category.

> - to call functions defined in the standard internal library, by using an
> + to call functions defined in the standard internal function library by using an
> interface similar to their SQL signature.
> Isn't it a bit redundant to say "internal function library" when we are already
> talking about function definitions?
>
> I agree that it may seem redundant, I added "function" here for the sake of consistency with lines 1829/1830 (if
appliedto the master branch) 
> where the documentation mentions "standard internal function library".

I hadn't seen that, but with that in mind I agree that being consistent is good
so I'll withdraw that comment.

--
Daniel Gustafsson

Dear Daniel,

Thank you for your prompt feedback.

Attached, please find the updated documentation patch, which incorporates your suggestions from both the first and second rounds of review.

--
Oleg Sibiryakov

On 13 Oct 2025, at 12:51, Oleg <o.sibiryakov@postgrespro.ru> wrote:

- <command>COPY</command> and other functions which allow executing a
+ the <command>COPY</command> command and functions, which allow executing a
I'm not sure about these, I think we use COPY without the the "the COPY
command" decoration in many places so I think it's more consistent like this.

I actually think we should add the decoration here because "<command>COPY</command> and other file-access functions"
sounds a bit confusing since COPY is not a file-access function and we seem to put it in the list. Even though I 
agree that everybody knows COPY is a command, not a function.

We refer to SQL commands by just their names all over the documentation without
saying "an EXPLAIN command" etc, and I think this falls in that same category.

- to call functions defined in the standard internal library, by using an
+ to call functions defined in the standard internal function library by using an
interface similar to their SQL signature.
Isn't it a bit redundant to say "internal function library" when we are already
talking about function definitions?

I agree that it may seem redundant, I added "function" here for the sake of consistency with lines 1829/1830 (if applied to the master branch)
where the documentation mentions "standard internal function library".

I hadn't seen that, but with that in mind I agree that being consistent is good
so I'll withdraw that comment.

--
Daniel Gustafsson

On 25.10.25 21:01, Oleg wrote:
> Dear Daniel,
> 
> Thank you for your prompt feedback.
> 
> Attached, please find the updated documentation patch, which 
> incorporates your suggestions from both the first and second rounds of 
> review.

       <term><literal>ALTER_REPLICATION_SLOT</literal> <replaceable 
class="parameter">slot_name</replaceable> ( <replaceable 
class="parameter">option</replaceable> [, ...] )
        <indexterm><primary>ALTER_REPLICATION_SLOT</primary></indexterm>
       </term>
       <listitem>
        <para>
-       Change the definition of a replication slot.
+       Changes the definition of a replication slot.

I think these are intentionally written in imperative style.  Compare 
the synopses of the main SQL commands: "change the definition of a 
domain" etc.

-     First, define a <literal>PgStat_KindInfo</literal> that includes all
+     First, define <literal>PgStat_KindInfo</literal> that includes all

I think this change is incorrect.  You are being asked to define an 
instance of PgStat_KindInfo, not PgStat_KindInfo itself (which already 
exists).

The remaining changes look ok to me.