Re: BUG #18280: logical decoding build wrong snapshot for subtransactions

On Wed, Jan 24, 2024 at 3:30 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> > While looking closely at the test result, I wondered how the following
> > part of test "s0_begin" "s0_truncate" "s1_checkpoint" "s1_get_changes"
> > "s0_insert_part" "s1_get_changes" "s0_commit" can see the insert when
> > the containing transaction is not yet committed. Then, I found it is
> > because the insert is performed from session 1 due to way it is
> > declared.
> >
> > session "s1"
> > ...
> > step "s1_get_changes" { SELECT data FROM
> > pg_logical_slot_get_changes('isolation_slot', NULL, NULL,
> > 'skip-empty-xacts', '1', 'include-xids', '0'); }
> > +step "s0_insert_part" { INSERT INTO tbl1_part VALUES (1); }
> >
> > I think this session should be performed from seesion-1 and we need
> > one more get_changes() call to see the problem. I have modified the
> > test accordingly and also changed the comments. See the attached and
> > let me know what you people think.
>
> Agreed your point and thanks for updating the patch.
> I confirmed your patch passed the test on both PG14 and PG15.
>
> One comment for commit message:
>
> >
>     This can happen when during restart, none of the WAL records from the
>     subtransaction was decoded and top-level xact contains a DDL.
> >
>
> It may be OK, the word "restart" may be confusing because it does not mean for
> the server instance. How about "decode" or "reply"?
>

Okay, we can change it to 'decoding'. I'll do this before commit
unless there are more comments. I'll push this early next week
(probably on Monday) unless I see any comments.

--
With Regards,
Amit Kapila.