On 08.12.21 01:23, Tomas Vondra wrote:
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.
Good point. I think it could be removed, but IIRC it'll be needed when
adding sequence decoding to the built-in replication, and that patch is
meant to be just an implementation of the API, without changing WAL.
OTOH I don't see it in the last version of that patch (in ResetSequence2
call) so maybe it's not really needed. I've kept it for now, but I'll
investigate.
Ok, please check. If it is needed, perhaps then we need a way for
test_decoding() to simulate it, for testing. But perhaps it's not needed.
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.
You mean in OutputPluginCallbacks? I'd actually argue it's the truncate
callbacks that are inconsistent - in the regular section truncate_cb is
right before commit_cb, while in the streaming section it's at the end.
Ok, that makes sense. Then leave yours.
When the question about fill_seq_with_data() is resolved, I have no more
comments on this part.
