Re: logical decoding and replication of sequences

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: logical decoding and replication of sequences
Date: 2022-01-27 16:05:59
Message-ID: 802f8b1b-a798-be52-c829-7f0fb9faafd5@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 27.01.22 00:32, Tomas Vondra wrote:
>
> On 1/26/22 14:01, Petr Jelinek wrote:
>> I would not remove it altogether, there is plenty of consumers of this
>> extension's output in the wild (even if I think it's
>> unfortunate) that might not be interested in sequences, but changing
>> it to off by default certainly makes sense.
>
> Indeed. Attached is an updated patch series, with 0003 switching it to
> false by default (and fixing the fallout). For commit I'll merge that
> into 0002, of course.

(could be done in separate patches too IMO)

test_decoding.c uses %zu several times for int64 values, which is not
correct. This should use INT64_FORMAT or %lld with a cast to (long long
int).

I don't know, maybe it's worth commenting somewhere how the expected
values in contrib/test_decoding/expected/sequence.out come about.
Otherwise, it's quite a puzzle to determine where the 3830 comes from,
for example.

I think the skip-sequences options is better turned around into a
positive name like include-sequences. There is a mix of "skip" and
"include" options in test_decoding, but there are more "include" ones
right now.

In the 0003, a few files have been missed, apparently, so the tests
don't fully pass. See attached patch.

I haven't looked fully through the 0004 patch, but I notice that there
was a confusing mix of FOR ALL SEQUENCE and FOR ALL SEQUENCES. I have
corrected that in the other attached patch.

Other than the mentioned cosmetic issues, I think 0001-0003 are ready to go.

Attachment Content-Type Size
0001-fixup-Change-skip-sequences-to-false-by-default.patch text/plain 5.8 KB
0003-fixup-Add-support-for-decoding-sequences-to-built-in.patch text/plain 1.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-01-27 16:47:47 Re: refactoring basebackup.c
Previous Message Alvaro Herrera 2022-01-27 15:15:27 Re: support for MERGE