Subject: Re: logical decoding and replication of sequences
To: Tomas Vondra <tomas.vondra@enterprisedb.com>,
 vignesh C <vignesh21@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Petr Jelinek <petr.jelinek@enterprisedb.com>
References: <d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com>
 <377c6fda-c3eb-7a69-e981-c77ae459c5db@enterprisedb.com>
 <bb890134-a566-efbc-f418-cd0f92eb4b70@enterprisedb.com>
 <5105c3ad-d726-fc22-1d09-514b6852a15e@enterprisedb.com>
 <CALDaNm3gZZjBKWs7gufQep9ikJMtoY9Xv4CsYTpYRaT720RpbQ@mail.gmail.com>
 <3005299b-267d-8e4a-68a3-abdad3e96aba@enterprisedb.com>
 <3d6df331-5532-6848-eb45-344b265e0238@enterprisedb.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Message-ID: <482c10c1-b110-819f-b0e7-17549fa2f0dc@enterprisedb.com>
Date: Thu, 23 Sep 2021 12:27:17 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <3d6df331-5532-6848-eb45-344b265e0238@enterprisedb.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/482c10c1-b110-819f-b0e7-17549fa2f0dc%40enterprisedb.com>
Precedence: bulk

On 30.07.21 20:26, Tomas Vondra wrote:
> Here's a an updated version of this patch - rebased to current master, 
> and fixing some of the issues raised in Peter's review.

This patch needs an update, as various conflicts have arisen now.

As was discussed before, it might be better to present a separate patch 
for just the logical decoding part for now, since the replication and 
DDL stuff has the potential to conflict heavily with other patches being 
discussed right now.  It looks like cutting this patch in two should be 
doable easily.

I looked through the test cases in test_decoding again.  It all looks 
pretty sensible.  If anyone can think of any other tricky or dubious 
cases, we can add them there.  It's easiest to discuss these things with 
concrete test cases rather than in theory.

One slightly curious issue is that this can make sequence values go 
backwards, when seen by the logical decoding consumer, like in the test 
case:

+ BEGIN
+ sequence: public.test_sequence transactional: 1 created: 1 last_value: 
1, log_cnt: 0 is_called: 0
+ COMMIT
+ sequence: public.test_sequence transactional: 0 created: 0 last_value: 
33, log_cnt: 0 is_called: 1
+ BEGIN
+ sequence: public.test_sequence transactional: 1 created: 1 last_value: 
4, log_cnt: 0 is_called: 1
+ COMMIT
+ sequence: public.test_sequence transactional: 0 created: 0 last_value: 
334, log_cnt: 0 is_called: 1

I suppose that's okay, since it's not really the intention that someone 
is concurrently consuming sequence values on the subscriber.  Maybe 
something for the future.  Fixing that would require changing the way 
transactional sequence DDL updates these values, so it's not directly 
the job of the decoding to address this.

A small thing I found: Maybe the text that test_decoding produces for 
sequences can be made to look more consistent with the one for tables. 
For example, in

+ BEGIN
+ sequence: public.test_table_a_seq transactional: 1 created: 1 
last_value: 1, log_cnt: 0 is_called: 0
+ sequence: public.test_table_a_seq transactional: 1 created: 0 
last_value: 33, log_cnt: 0 is_called: 1
+ table public.test_table: INSERT: a[integer]:1 b[integer]:100
+ table public.test_table: INSERT: a[integer]:2 b[integer]:200
+ COMMIT

note how the punctuation is different.