Date: Mon, 20 Dec 2021 20:11:04 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: Confused comment about drop replica identity index
Message-ID: <YcBkyHOBwSljDkCZ@paquier.xyz>
References: 
 <OS3PR01MB6275464AD0A681A0793F56879E759@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <CAExHW5uWwcV0-=JQXYhEpnEtmwmZQsh0c1N3cxxBYRhwE9WuvQ@mail.gmail.com>
 <Yblf/R3h0Dj+Ui2v@paquier.xyz>
 <OS3PR01MB6275A10D03572E27C4E2C9769E769@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <YbpuuHqAkf2hjOA0@paquier.xyz>
 <OS3PR01MB62750A124D384B4E96CA43F19E779@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <OS3PR01MB62750A0A1A282E9E0148172B9E7B9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="C3KObG+dG5/GCVsd"
Content-Disposition: inline
In-Reply-To: 
 <OS3PR01MB62750A0A1A282E9E0148172B9E7B9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YcBkyHOBwSljDkCZ%40paquier.xyz>
Precedence: bulk


--C3KObG+dG5/GCVsd
Content-Type: multipart/mixed; boundary="GRO7Xxh9t5V5i0Lw"
Content-Disposition: inline


--GRO7Xxh9t5V5i0Lw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Dec 20, 2021 at 03:46:13AM +0000, wangw.fnst@fujitsu.com wrote:
> Here is a patch to correct wrong comment about
> REPLICA_IDENTITY_INDEX, And improve the pg-doc.

That's mostly fine.  I have made some adjustments as per the
attached.

+         The default for non-system tables. Records the old values of the =
columns
+         of the primary key, if any. The default for non-system tables.=20
The same sentence is repeated twice.

+         Records no information about the old row.(This is the
default for system tables.)
For consistency with the rest, this could drop the parenthesis for the
second sentence.

+       <term><literal>USING INDEX index_name</literal></term>
This should use <replaceable> as markup for index_name.

Pondering more about this thread, I don't think we should change the
existing behavior in the back-branches, but I don't have any arguments
about doing such changes on HEAD to help the features being worked
on, either.  So I'd like to apply and back-patch the attached, as a
first step, to fix the inconsistency.
--
Michael

--GRO7Xxh9t5V5i0Lw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-Correct-comment-and-documentation-about-REPLICA_I.patch"
Content-Transfer-Encoding: quoted-printable

=46rom 05fe07224a5f31f3d28da088f302772724c00dd4 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Mon, 20 Dec 2021 20:09:59 +0900
Subject: [PATCH v2] Correct comment and documentation about
 REPLICA_IDENTITY_INDEX

---
 src/include/catalog/pg_class.h    |  2 +-
 doc/src/sgml/ref/alter_table.sgml | 51 ++++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 93338d267c..b46299048e 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -182,7 +182,7 @@ DECLARE_INDEX(pg_class_tblspc_relfilenode_index, 3455, =
ClassTblspcRelfilenodeInd
 /*
  * an explicitly chosen candidate key's columns are used as replica identi=
ty.
  * Note this will still be set if the index has been dropped; in that case=
 it
- * has the same meaning as 'd'.
+ * has the same meaning as 'n'.
  */
 #define		  REPLICA_IDENTITY_INDEX	'i'
=20
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_tab=
le.sgml
index 8f14e4a5c4..a76e2e7322 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -872,16 +872,51 @@ WITH ( MODULUS <replaceable class=3D"parameter">numer=
ic_literal</replaceable>, REM
      <para>
       This form changes the information which is written to the write-ahea=
d log
       to identify rows which are updated or deleted.  This option has no e=
ffect
-      except when logical replication is in use.  <literal>DEFAULT</litera=
l>
-      (the default for non-system tables) records the
-      old values of the columns of the primary key, if any.  <literal>USIN=
G INDEX</literal>
-      records the old values of the columns covered by the named index, wh=
ich
-      must be unique, not partial, not deferrable, and include only column=
s marked
-      <literal>NOT NULL</literal>.  <literal>FULL</literal> records the ol=
d values of all columns
-      in the row.  <literal>NOTHING</literal> records no information about=
 the old row.
-      (This is the default for system tables.)
+      except when logical replication is in use.
       In all cases, no old values are logged unless at least one of the co=
lumns
       that would be logged differs between the old and new versions of the=
 row.
+     <variablelist>
+      <varlistentry>
+       <term><literal>DEFAULT</literal></term>
+       <listitem>
+        <para>
+         Records the old values of the columns of the primary key, if any.
+         This is the default for non-system tables.=20
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><literal>USING INDEX <replaceable class=3D"parameter">index_n=
ame</replaceable></literal></term>
+       <listitem>
+        <para>
+         Records the old values of the columns covered by the named index,
+         that must be unique, not partial, not deferrable, and include only
+         columns marked <literal>NOT NULL</literal>. If this index is
+         dropped, the behavior is the same as <literal>NOTHING</literal>.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><literal>FULL</literal></term>
+       <listitem>
+        <para>
+         Records the old values of all columns in the row.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><literal>NOTHING</literal></term>
+       <listitem>
+        <para>
+         Records no information about the old row. This is the default for
+         system tables.
+        </para>
+       </listitem>
+      </varlistentry>
+     </variablelist>
      </para>
     </listitem>
    </varlistentry>
--=20
2.34.1


--GRO7Xxh9t5V5i0Lw--

--C3KObG+dG5/GCVsd
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmHAZMgACgkQnvQgOdby
QH1NaA/+J1Obj/b6U7YlM0LZ4VMLXuRHmOFRIRQeu72s0X3WGpGpwAoqCPaYMD0G
jlT3NIoDOGkqUdilYory91EEjWjzBbtCXMzIOzUM9NswTaH1MURCoN9FzgbtcZ0y
sSkIwY9z3QKprKwfYFr2A0BHzHut5+KqvP59hbQzrl7B59RrJVbtwneyj3BE4D+q
icJ4k5i+Z+8OZxpQuAStv8B6D62SOoi8sALJpd65QUPfTK/7+TVkscbBYz/BpBv4
74LMkr48LS8TfT8sllNPFnfaLFBavh8pHHTUTxWXdrV3psx5bMqfMFEJy43wGkgv
keCW9SDCnWdNqi4iaoiMwN1wjQ1N91+VhgeUhGJ3gesftq5DiQg68Yo7Q37dtxkt
2MJzv3wurorkVRVBTnjj/8twlSG0qFTb9FiE8BLjuFChWuM2J4V2qwDpTA1M838M
YnA5fzqZ+uofSpLpM812KtrgJkSF0Me6d9znPNl7La8kF70ahmO+GFJqS68QGS7Y
5FJfWk9BiX6x7U45NSzMYSyLtYeh6Cp/YIOXuramyy0i7HOb3e2xGqqyxxgoy7gd
hbrtCveJnTOKyNQBTOSzGtdlAyHAokvz6vpsFwugz4ydMygbXML/UwjCFxpZu0CI
P+xmLvtMbpN2ueQhIJcuvhLWTWhUW9Aq+v9LdBi5K+E50z6bSlY=
=LrUt
-----END PGP SIGNATURE-----

--C3KObG+dG5/GCVsd--