Re: logical decoding and replication of sequences

On Fri, Mar 25, 2022 at 10:20 PM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> Hmm, so fixing this might be a bit trickier than I expected.
>
> Firstly, currently we only send nspname/relname in the sequence message,
> not the remote OID or schema. The idea was that for sequences we don't
> really need schema info, so this seemed OK.
>
> But should_apply_changes_for_rel() needs LogicalRepRelMapEntry, and to
> create/maintain that those records we need to send the schema.
>
> Attached is a WIP patch does that.
>
> Two places need more work, I think:
>
> 1) maybe_send_schema needs ReorderBufferChange, but we don't have that
> for sequences, we only have TXN. I created a simple wrapper, but maybe
> we should just tweak maybe_send_schema to use TXN.
>
> 2) The transaction handling in is a bit confusing. The non-transactional
> increments won't have any explicit commit later, so we can't just rely
> on begin_replication_step/end_replication_step. But I want to try
> spending a bit more time on this.
>

I didn't understand what you want to say in point (2).

>
> But there's a more serious issue, I think. So far, we allowed this:
>
> BEGIN;
> CREATE SEQUENCE s2;
> ALTER PUBLICATION p ADD SEQUENCE s2;
> INSERT INTO seq_test SELECT nextval('s2') FROM generate_series(1,100);
> COMMIT;
>
> and the behavior was that we replicated the changes. But with the patch
> applied, that no longer happens, because should_apply_changes_for_rel
> says the change should not be applied.
>
> And after thinking about this, I think that's correct - we can't apply
> changes until ALTER SUBSCRIPTION ... REFRESH PUBLICATION gets executed,
> and we can't do that until the transaction commits.
>
> So I guess that's correct, and the current behavior is a bug.
>

Yes, I also think that is a bug.

> For a while I was thinking that maybe this means we don't need the
> transactional behavior at all, but I think we do - we have to handle
> ALTER SEQUENCE cases that are transactional.
>

I need some time to think about this. At all places, it is mentioned
as creating a sequence for transactional cases which at the very least
need some tweak.

--
With Regards,
Amit Kapila.