HI Hous-San,. Here is my review of the v20-0001 docs patch.
1. Restructure into sections
> I think that's a good idea. But I preferred to do that in a separate
> patch(maybe a third patch after the first and second are RFC), because AFAICS
> we would need to adjust some existing docs which falls outside the scope of
> the current patch.
OK. I thought deferring it would only make extra work/churn, given you
already know up-front that everything will require restructuring later
anyway.
~~~
2. Synopsis
2.1 synopsis wrapping.
> I thought about this, but wrapping the sentence would cause the words
> to be displayed in different lines after compiling. I think that's
> inconsistent
> with the real log which display the tuples in one line.
IMO the readability of the format is the most important objective for
the documentation. And, as you told Shveta, there is already a real
example where people can see the newlines if they want to.
nit - Anyway, FYI there is are newline rendering problems here in v20.
Removed newlines to make all these optional parts appear on the same
line.
2.2 other stuff
nit - Add underscore to /detailed explanation/detailed_explanation/,
to make it more obvious this is a replacement parameter
nit - Added newline after </synopsis> for readabilty of the SGML file.
~~~
3. Case of literals
It's not apparent to me why the optional "Key" part should be
uppercase in the LOG but other (equally important?) literals of other
parts like "replica identity" are not.
It seems inconsistent.
~~~
4. LOG parts
nit - IMO the "schema.tablename" and the "conflict_type" deserved to
have separate listitems.
nit - The "conflict_type" should have <replaceable> markup.
~~~
5. DETAIL parts
nit - added newline above this <varlistentry> for readability of the SGML.
nit - Add underscore to detailed_explanation, and rearrange wording to
name the parameter up-front same as the other bullets do.
nit - change the case /key/Key/ to match the synopsis.
~~~
6.
+ <para>
+ The <literal>replica identity</literal> section includes the replica
+ identity key values that used to search for the existing
local tuple to
+ be updated or deleted. This may include the full tuple value
if the local
+ relation is marked with <literal>REPLICA IDENTITY FULL</literal>.
+ </para>
It might be good to also provide a link for that REPLICA IDENTITY
FULL. (I did this already in the attachment as an example)
~~~
7. Replacement parameters - column_name, column_value
I've included these for completeness. I think it is useful.
BTW, the column names seem sometimes optional but I did not know the
rules. It should be documented what makes these names be shown or not
shown.
~~~
Please see the attachment which implements most of the items mentioned above.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/doc/src/sgml/logical-replication.sgml
b/doc/src/sgml/logical-replication.sgml
index 3df791a..a3a0eae 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -1670,11 +1670,10 @@ test_sub=# SELECT * FROM t1 ORDER BY id;
The log format for logical replication conflicts is as follows:
<synopsis>
LOG: conflict detected on relation
"<replaceable>schemaname</replaceable>.<replaceable>tablename</replaceable>":
conflict=<replaceable>conflict_type</replaceable>
-DETAIL: <replaceable class="parameter">detailed explanation</replaceable>.
-<optional>
-<literal>Key</literal> (<replaceable>column_name</replaceable>,
...)=(<replaceable>column_value</replaceable>, ...)</optional>
-<optional>; <literal>existing local tuple</literal>
<optional>(<replaceable>column_name</replaceable>,
...)=</optional>(<replaceable>column_value</replaceable>,
...)</optional><optional>; <literal>remote tuple</literal>
<optional>(<replaceable>column_name</replaceable>,
...)=</optional>(<replaceable>column_value</replaceable>,
...)</optional><optional>; <literal>replica identity</literal>
{(<replaceable>column_name</replaceable>,
...)=(<replaceable>column_value</replaceable>, ...) | full
(<replaceable>column_value</replaceable>, ...)}</optional>.
+DETAIL: <replaceable class="parameter">detailed_explanation</replaceable>.
+<optional><literal>Key</literal> (<replaceable>column_name</replaceable>,
...)=(<replaceable>column_value</replaceable>, ...)</optional><optional>;
<literal>existing local tuple</literal>
<optional>(<replaceable>column_name</replaceable>,
...)=</optional>(<replaceable>column_value</replaceable>,
...)</optional><optional>; <literal>remote tuple</literal>
<optional>(<replaceable>column_name</replaceable>,
...)=</optional>(<replaceable>column_value</replaceable>,
...)</optional><optional>; <literal>replica identity</literal>
{(<replaceable>column_name</replaceable>,
...)=(<replaceable>column_value</replaceable>, ...) | full
(<replaceable>column_value</replaceable>, ...)}</optional>.
</synopsis>
+
The log provides the following information:
<variablelist>
<varlistentry>
@@ -1683,28 +1682,34 @@ DETAIL: <replaceable class="parameter">detailed
explanation</replaceable>.
<itemizedlist>
<listitem>
<para>
- The name of the local relation involved in the conflict and the
conflict
- type (e.g., <literal>insert_exists</literal>,
- <literal>update_exists</literal>).
+
<replaceable>schemaname</replaceable>.<replaceable>tablename</replaceable>
+ identifies the local relation involved in the conflict.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <replaceable>conflict_type</replaceable> is the type of conflict that
occurred
+ (e.g., <literal>insert_exists</literal>,
<literal>update_exists</literal>).
</para>
</listitem>
</itemizedlist>
</listitem>
</varlistentry>
+
<varlistentry>
<term><literal>DETAIL</literal></term>
<listitem>
<itemizedlist>
<listitem>
<para>
- The origin, transaction ID, and commit timestamp of the transaction
that
- modified the existing local tuple, if available, are included in the
- <replaceable class="parameter">detailed explanation</replaceable>.
+ <replaceable class="parameter">detailed_explanation</replaceable>
includes
+ the origin, transaction ID, and commit timestamp of the transaction
that
+ modified the existing local tuple, if available.
</para>
</listitem>
<listitem>
<para>
- The <literal>key</literal> section includes the key values of the
local
+ The <literal>Key</literal> section includes the key values of the
local
tuple that violated a unique constraint
<literal>insert_exists</literal>
or <literal>update_exists</literal> conflicts.
</para>
@@ -1732,7 +1737,20 @@ DETAIL: <replaceable class="parameter">detailed
explanation</replaceable>.
The <literal>replica identity</literal> section includes the replica
identity key values that used to search for the existing local tuple
to
be updated or deleted. This may include the full tuple value if the
local
- relation is marked with <literal>REPLICA IDENTITY FULL</literal>.
+ relation is marked with
+ <link linkend="sql-altertable-replica-identity-full"><literal>REPLICA
IDENTITY FULL</literal></link>.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <replaceable class="parameter">column_name</replaceable> is the
column name.
+ These are optionally logged and, if present, are in the same order as
the
+ corresponding column value.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <replaceable class="parameter">column_value</replaceable> is the
column value.
</para>
</listitem>
</itemizedlist>
