On Thu, Sep 10, 2020 at 12:00 PM Dilip Kumar <dilipbalaut@gmail.com> wrote: > > On Thu, Sep 10, 2020 at 11:53 AM Dilip Kumar <dilipbalaut@gmail.com> wrote: >> >>> > >>> > I have written a test case to reproduce the same. >
Can we write an isolation test for this scenario? See some similar tests in contrib/test_decoding/specs. If that is possible then we can probably remove the test which failed and instead write an isolation test involving three transactions as shown by you. Also, please prepare two separate patches (one for test and other for code) if you are able to convert existing test to an isolation test as that will make it easier to test the fix.
I have written a test in isolation test. IMHO, we should not try to merge stream.sql to this isolation test mainly for two reasons a) this isolation test is very specific that while we are trying to stream we must have the incomplete changes so if we try to put more operation like message/truncate/abort then it will become unpredictable. Currently, I have kept it with just one big tuple so it is a guarantee that whenever the the logical_decoding_work_mem exceed then it will have the partial changes. b) we can add another operation in the transaction and cover the stream changes but then those are not very specific to the isolation test. So I feel it is better to put only the specific scenario in the isolation test.
> > I have removed some comments which are not valid after this patch. >
Few comments: ============= 1. We need to set xact_wrote_changes in pg_decode_stream_truncate() as well along with the APIs in which you have set it. 2. +static void +pg_output_stream_start(LogicalDecodingContext *ctx, TestDecodingData *data, ReorderBufferTXN *txn, bool last_write) +{ OutputPluginPrepareWrite(ctx, true); if (data->include_xids) appendStringInfo(ctx->out, "opening a streamed block for transaction TXN %u", txn->xid); @@ -601,16 +610,15 @@ pg_decode_stream_start(LogicalDecodingContext *ctx, OutputPluginWrite(ctx, true);
In this API, we need to use 'last_write' in OutputPluginPrepareWrite() and OutputPluginWrite().
The attached patch fixes both these comments.
Okay, there is some change in stream.out so I have included that in the first patch.