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>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com>
Subject: Re: logical decoding and replication of sequences
Date: 2021-12-07 14:38:50
Message-ID: c0bd317a-fef5-9ef9-66be-a10eae686b96@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I have checked the 0001 and 0003 patches. (I think we have dismissed
the approach in 0002 for now. And let's talk about 0004 later.)

I have attached a few fixup patches, described further below.

# 0001

The argument "create" for fill_seq_with_data() is always true (and
patch 0002 removes it). So I'm not sure if it's needed. If it is, it
should be documented somewhere.

About the comment you added in nextval_internal(): It's a good
explanation, so I would leave it in. I also agree with the
conclusion of the comment that the current solution is reasonable. We
probably don't need the same comment again in fill_seq_with_data() and
again in do_setval(). Note that both of the latter functions already
point to nextval_interval() for other comments, so the same can be
relied upon here as well.

The order of the new fields sequence_cb and stream_sequence_cb is a
bit inconsistent compared to truncate_cb and stream_truncate_cb.
Maybe take another look to make the order more uniform.

Some documentation needs to be added to the Logical Decoding chapter.
I have attached a patch that I think catches all the places that need
to be updated, with some details left for you to fill in. ;-) The
ordering of the some of the documentation chunks reflects the order in
which the callbacks appear in the header files, which might not be
optimal; see above.

In reorderbuffer.c, you left a comment about how to access a sequence
tuple. There is an easier way, using Form_pg_sequence_data, which is
how sequence.c also does it. See attached patch.

# 0003

The tests added in 0003 don't pass for me. It appears that you might
have forgotten to update the expected files after you added some tests
or something. The cfbot shows the same. See attached patch for a
correction, but do check what your intent was.

As mentioned before, we probably don't need the skip-sequences option
in test_decoding.

Attachment Content-Type Size
0001-Whitespace-fixes.patch text/plain 2.8 KB
0002-Make-tests-pass.patch text/plain 2.4 KB
0003-Some-documentation-updates.patch text/plain 5.6 KB
0004-Simplify-accessing-sequence-tuple-data.patch text/plain 3.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-12-07 14:44:18 Re: Skipping logical replication transactions on subscriber side
Previous Message Bharath Rupireddy 2021-12-07 14:36:22 Is there a way (except from server logs) to know the kind of on-going/last checkpoint?