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

Dear Amit,

> 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"?
If you do not like, it's OK to keep.

Others, LGTM.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED