Обсуждение: pg_wal_lsn_diff

According to the pg_wal_lsn_diff manual:

pg_wal_lsn_diff ( lsn pg_lsn, lsn pg_lsn ) → numeric

    Calculates the difference in bytes between two write-ahead log
    locations. This can be used with pg_stat_replication or some of
    the functions shown in Table 9.87 to get the replication lag.

It's not clear whether its return value is (first arg - second arg) or
(second arg - first arg). The order matters because the return value
could have a sign (it actually does first arg - second arg). Attached
is a patch to fix the description.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b7150510ab..2870692164 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25102,11 +25102,11 @@ SELECT collation for ('foo' COLLATE "de_DE");
         <indexterm>
          <primary>pg_wal_lsn_diff</primary>
         </indexterm>
-        <function>pg_wal_lsn_diff</function> ( <parameter>lsn</parameter> <type>pg_lsn</type>,
<parameter>lsn</parameter><type>pg_lsn</type> )
 
+        <function>pg_wal_lsn_diff</function> ( <parameter>lsn1</parameter> <type>pg_lsn</type>,
<parameter>lsn2</parameter><type>pg_lsn</type> )
 
         <returnvalue>numeric</returnvalue>
        </para>
        <para>
-        Calculates the difference in bytes between two write-ahead log
+        Calculates the difference (ls1 - ls2) in bytes between two write-ahead log
         locations.  This can be used
         with <structname>pg_stat_replication</structname> or some of the
         functions shown in <xref linkend="functions-admin-backup-table"/> to

Tatsuo Ishii <ishii@sraoss.co.jp> writes:
> According to the pg_wal_lsn_diff manual:
> pg_wal_lsn_diff ( lsn pg_lsn, lsn pg_lsn ) $B"*(B numeric

>     Calculates the difference in bytes between two write-ahead log
>     locations. This can be used with pg_stat_replication or some of
>     the functions shown in Table 9.87 to get the replication lag.

> It's not clear whether its return value is (first arg - second arg) or
> (second arg - first arg). The order matters because the return value
> could have a sign (it actually does first arg - second arg). Attached
> is a patch to fix the description.

+1 for fixing that.  Two notes though: first, your text doesn't agree
with the prototype ("ls1" should be "lsn1"), and second, I think it
would read better if you didn't split the phrase "difference in bytes".
That is,

-        Calculates the difference in bytes between two write-ahead log
+        Calculates the difference in bytes (lsn1 - lsn2) between two write-ahead log

Also maybe you should use the <parameter>lsn1</parameter> markup
in the text.

            regards, tom lane

> +1 for fixing that.  Two notes though: first, your text doesn't agree
> with the prototype ("ls1" should be "lsn1"), and second, I think it
> would read better if you didn't split the phrase "difference in bytes".
> That is,
> 
> -        Calculates the difference in bytes between two write-ahead log
> +        Calculates the difference in bytes (lsn1 - lsn2) between two write-ahead log
> 
> Also maybe you should use the <parameter>lsn1</parameter> markup
> in the text.

Thank you for the review. Attached is the revised patch.

So do you think we should back-patch to existing stable branches?  The
reason I am asking is, the doc format for v12 and before use very
simple description and if we apply the same description of this patch
to those branche, the description level for the function may look
different from other functions.

(I am not against to apply to v13).

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b7150510ab..1ab31a9056 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25102,11 +25102,11 @@ SELECT collation for ('foo' COLLATE "de_DE");
         <indexterm>
          <primary>pg_wal_lsn_diff</primary>
         </indexterm>
-        <function>pg_wal_lsn_diff</function> ( <parameter>lsn</parameter> <type>pg_lsn</type>,
<parameter>lsn</parameter><type>pg_lsn</type> )
 
+        <function>pg_wal_lsn_diff</function> ( <parameter>lsn1</parameter> <type>pg_lsn</type>,
<parameter>lsn2</parameter><type>pg_lsn</type> )
 
         <returnvalue>numeric</returnvalue>
        </para>
        <para>
-        Calculates the difference in bytes between two write-ahead log
+        Calculates the difference in bytes (<parameter>lsn1</parameter> - <parameter>lsn2</parameter>) between two
write-aheadlog
 
         locations.  This can be used
         with <structname>pg_stat_replication</structname> or some of the
         functions shown in <xref linkend="functions-admin-backup-table"/> to

Tatsuo Ishii <ishii@sraoss.co.jp> writes:
> So do you think we should back-patch to existing stable branches?

I'd just fix HEAD and v13.  The earlier branches are not friendly
at all to long function descriptions.

            regards, tom lane

> I'd just fix HEAD and v13.  The earlier branches are not friendly
> at all to long function descriptions.

+1 from me.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

>> I'd just fix HEAD and v13.  The earlier branches are not friendly
>> at all to long function descriptions.
> 
> +1 from me.

Pushed to HEAD and v13.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp