On Thu, Jun 10, 2021 at 2:04 PM Ajin Cherian <itsa...@gmail.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.