Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

On Thu, Jun 10, 2021 at 2:04 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>

The new patches look mostly good apart from the below cosmetic issues.
I think the question is whether we want to do these for PG-14 or
postpone them till PG-15. I think these don't appear to be risky
changes so we can get them in PG-14 as that might help some outside
core solutions as appears to be the case for Jeff. The changes related
to start_replication are too specific to the subscriber-side solution
so we can postpone those along with the subscriber-side 2PC work.
Jeff, Ajin, what do you think?

Also, I can take care of the below cosmetic issues before committing
if we decide to do this for PG-14.

Few cosmetic issues:
==================
1. git diff --check shows
src/bin/pg_basebackup/t/030_pg_recvlogical.pl:109: new blank line at EOF.

2.
+
<para>
The following example shows SQL interface that can be used to decode prepared
transactions. Before you use two-phase commit commands, you must set

Spurious line addition.

3.
/* Build query */
appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\"", slot_name);
if (is_temporary)
appendPQExpBufferStr(query, " TEMPORARY");
+
if (is_physical)

Spurious line addition.

4.
appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
+ if (two_phase && PQserverVersion(conn) >= 140000)
+ appendPQExpBufferStr(query, " TWO_PHASE");
+
if (PQserverVersion(conn) >= 100000)
/* pg_recvlogical doesn't use an exported snapshot, so suppress */
appendPQExpBufferStr(query, " NOEXPORT_SNAPSHOT");

I think it might be better to append TWO_PHASE after NOEXPORT_SNAPSHOT
but it doesn't matter much.

5.
+$node->safe_psql('postgres',
+ "BEGIN;INSERT INTO test_table values (11); PREPARE TRANSACTION 'test'");

There is no space after BEGIN but there is a space after INSERT. For
consistency-sake, I will have space after BEGIN as well.

--
With Regards,
Amit Kapila.