Re: Conflict detection and logging in logical replication

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