Horton

Re: logical decoding and replication of sequences

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

View thread View on postgr.es
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Petr Jelinek <petr.jelinek@enterprisedb.com>
Date: 2021-12-07T14:38:50Z
Lists: pgsql-hackers

Attachments

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.

Commits

  1. Fix cache invalidation bug in recovery_prefetch.

  2. Revert "Logical decoding of sequences"

  3. Minor improvements in sequence decoding code and docs

  4. Handle sequences in preprocess_pubobj_list

  5. Update tab-completion for CREATE PUBLICATION with sequences

  6. Add decoding of sequences to built-in replication

  7. Stabilize test_decoding touching with sequences

  8. Call ReorderBufferProcessXid from sequence_decode

  9. Add decoding of sequences to test_decoding

  10. Replace Test::More plans with done_testing

  11. Logical decoding of sequences

  12. Respect permissions within logical replication.