Follow-up review items for update_deleted

On Wed, Sep 2, 2026 at 10:00 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

Nisha helped perform a thorough review of the update_deleted feature using
Claude and shared the findings with me off-list. After some analysis and
discussion, we identified a few items worth addressing (fixes are
attached (0001-0004)). Please see the details below:

1. Slot xmin backward movement with multiple subscriptions

I'll review this.

See 0001 for the fix and test.

2. Premature slot advancement with asynchronous commit

...

3

Missing trailing periods to update_deleted conflict DETAILs. Simple fix in 0004.

The fix for 2 and 3 looks good to me, so I have pushed those after
changing the comments slightly.

--
Other items (for recording only)
--

The following items were also reported but don't seem worth changing at this
point. They can be revisited later if needed - sharing them here for reference.

4

With track_commit_timestamp off and retain_dead_tuples on,
FindDeletedTupleInLocalRel() returns false unconditionally and silently, while
dead tuples are still retained.

This is not considered as a bug, as the documentation clearly states that
update_deleted requires track_commit_timestamp to be enabled, and we emit a
WARNING when retain_dead_tuples is enabled without it. If needed, we could
improve this by stopping retention when track_commit_timestamp is disabled, but
disabling track_commit_timestamp while retain_dead_tuples is enabled seems like
an extremely unlikely user behavior. So would be better to wait for user
feedback before taking any action.

5

max_retention_duration does not work for a disabled subscription or a
keep-failing worker, meaning retention cannot be stopped using this option in
those cases.

This is documented behavior (and user can disable the retain_dead_tuples
manually):

This option is effective only when retain_dead_tuples is enabled and the apply
worker associated with the subscription is active.

Both 4 and 5 are documented but will it be better to provide this
information explicitly, say via Logging when the conflcit_slot is not
getting advanced due to any of these reasons?

--
With Regards,
Amit Kapila.

Dear Hou,

Thanks for the patch. I reviewed 0001 and could not find critical issues.
Few comments;

01.
```
/* Remember the retained databases for the next cycle. */
current_dbids = lappend_oid(current_dbids, sub->dbid);
```

list_append_unique_oid() is usable here.

02. init_conflict_slot_xmin()

Not sure the function name is good because it can be called many times.
How about: reset_conflict_slot_xmin_to_safe_horizon()? Code comment should also
be updated.

03.
```
/* Write this slot to disk */
ReplicationSlotMarkDirty();
ReplicationSlotSave();
```

IIUC they are needed only if the slot is updated.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Wed, Sep 2, 2026 at 10:00 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

Hi,

Nisha helped perform a thorough review of the update_deleted feature using
Claude and shared the findings with me off-list. After some analysis and
discussion, we identified a few items worth addressing (fixes are
attached (0001-0004)). Please see the details below:

1. Slot xmin backward movement with multiple subscriptions

Since the retain_dead_tuples feature maintains one replication slot for all
subscriptions, if two subscriptions are created at different times, the later
one may cause slot.xmin to move backwards.

For example, consider two subscriptions for databases A and B. The oldest XID
in database A is 700, while the oldest XID in database B is 500 (due to a
long-running transaction in that database). If subscription A is created first,
conflict_detection_slot.xmin advances to 700. When subscription B is later
created, it sees that the oldest XID in its own database is 500 ? which is older
than the current slot.xmin. This currently causes an Assert failure in the apply
worker. If the Assert is removed, it would allow slot.xmin to move backwards.

Moving slot.xmin backwards is actually correct behavior for subscription of
database B: the long-running transaction in database B is a candidate that could
generate dead tuples needed for update_deleted conflict detection, so slot.xmin
should not advance beyond that XID. However, instead of letting the worker
handle this (which could delay the slot.xmin update), we should have the
launcher detect the new subscription and adjust the slot directly.

The patch fix this by tracking the set of databases with actively-retaining
subscriptions in the launcher, and when a database newly appears in the set,
re-initialize the slot's xmin to the cluster-wide safe decoding horizon before
launching any workers. The horizon accounts for all running transactions
cluster-wide, so it is a safe seed for every database.

See 0001 for the fix and test.

Hi Hou-san,

Thanks for the fix patches.

I’ve reviewed and tested the Issue-1 fix (patch-001), and it LGTM. I
didn’t find any critical issues with the fix.

Just one test comment on [035_conflicts.pl:699]:

$node_B->wait_for_subscription_sync($node_A, $subname_BA2);

If the subscription is in a db other than postgres, we need to pass
the db name here. Otherwise, it connects to the default postgres db,
and the test could pass without actually checking the intended
subscription.

I think this should be:

$node_B->wait_for_subscription_sync($node_A, $subname_BA2, 'dbb');

--
Thanks,
Nisha

The patch 001 looks good to me. I had just one comment which I noticed
Kuroda-San has already covered.

ReplicationSlotMarkDirty();
ReplicationSlotSave();
These calls are only needed if xmin is adjusted.
--

Now, the better names for 'init_conflict_slot_xmin' could be:
adjust_conflict_slot_xmin, refresh_conflict_slot_xmin.

--

Also would it be better if we change elog to:

if (!TransactionIdIsValid(old_xmin))
elog(DEBUG1, "initialized conflict detection slot's xmin to %u", xmin_horizon);
else
elog(DEBUG1, "adjusted conflict detection slot's xmin from %u to %u",
old_xmin, xmin_horizon);

<Do it only if you find it better>

thanks
Shveta

On Thursday, September 3, 2026 9:57 PM shveta malik <shveta.malik@gmail.com> wrote:

The patch 001 looks good to me. I had just one comment which I noticed
Kuroda-San has already covered.

Thanks for the comments.

ReplicationSlotMarkDirty();
ReplicationSlotSave();
These calls are only needed if xmin is adjusted.

Changed.

--

Now, the better names for 'init_conflict_slot_xmin' could be:
adjust_conflict_slot_xmin, refresh_conflict_slot_xmin.

I chose Kuroda-San's version in this version.

--

Also would it be better if we change elog to:

if (!TransactionIdIsValid(old_xmin))
elog(DEBUG1, "initialized conflict detection slot's xmin to %u", xmin_horizon);
else elog(DEBUG1, "adjusted conflict detection slot's xmin from %u to %u",
old_xmin, xmin_horizon);

I added a single elog(DEBUG without an if condition to keep it simple.
Users can already see the history from the log anyway.

Apart from above comments, I also addressed comments from Kuroda-San[1]/messages/by-id/OS9PR01MB1214975F154718013E1B82D4AF5B62@OS9PR01MB12149.jpnprd01.prod.outlook.com and
Nisha[2]/messages/by-id/CABdArM7nwKpoVQ5v42xz6D-qAVrNGdb+yMUGmSR8Tapv=mFzDw@mail.gmail.com.

[1]: /messages/by-id/OS9PR01MB1214975F154718013E1B82D4AF5B62@OS9PR01MB12149.jpnprd01.prod.outlook.com
[2]: /messages/by-id/CABdArM7nwKpoVQ5v42xz6D-qAVrNGdb+yMUGmSR8Tapv=mFzDw@mail.gmail.com

Best Regards,
Zhijie Hou

On Fri, Sep 4, 2026 at 7:42 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

On Thursday, September 3, 2026 9:57 PM shveta malik <shveta.malik@gmail.com> wrote:

The patch 001 looks good to me. I had just one comment which I noticed
Kuroda-San has already covered.

Thanks for the comments.

ReplicationSlotMarkDirty();
ReplicationSlotSave();
These calls are only needed if xmin is adjusted.

Changed.

--

Now, the better names for 'init_conflict_slot_xmin' could be:
adjust_conflict_slot_xmin, refresh_conflict_slot_xmin.

I chose Kuroda-San's version in this version.

--

Also would it be better if we change elog to:

if (!TransactionIdIsValid(old_xmin))
elog(DEBUG1, "initialized conflict detection slot's xmin to %u", xmin_horizon);
else elog(DEBUG1, "adjusted conflict detection slot's xmin from %u to %u",
old_xmin, xmin_horizon);

I added a single elog(DEBUG without an if condition to keep it simple.
Users can already see the history from the log anyway.

Apart from above comments, I also addressed comments from Kuroda-San[1] and
Nisha[2].

[1] /messages/by-id/OS9PR01MB1214975F154718013E1B82D4AF5B62@OS9PR01MB12149.jpnprd01.prod.outlook.com
[2] /messages/by-id/CABdArM7nwKpoVQ5v42xz6D-qAVrNGdb+yMUGmSR8Tapv=mFzDw@mail.gmail.com

I have no further comments on the patch.

thanks
Shveta

On Fri, Sep 4, 2026 at 7:42 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

Apart from above comments, I also addressed comments from Kuroda-San[1] and
Nisha[2].

Thanks for the updated patch.

I was testing/reviewing the retention stop-resume path when two dbs
(say db1 and db2) are involved. I see a race condition between the
launcher and the worker that leads to the same assert.
When a sub resumes retention on db2, update_retention_status(true)
sets subretentionactive = true and calls ApplyLauncherWakeup() before
the worker restarts itself through apply_worker_exit().

Now if the launcher runs its next cycle while the old worker's entry
is still in_use(say we hold it just before apply_worker_exit), then in
that cycle, launcher will -
- add the db2 to retained_dbids.
- fire the reset because db2 is not yet in the set.
- Finds w != NULL (still old worker), so
compute_min_nonremovable_xid() reads the "InvalidTransactionId" left
by the earlier stop and returns without contributing to the result.

Now the situation is that update_conflict_slot_xmin() advances the
slot to the horizon of db1 (which can be way ahead of db2's horizon).
When eventually a new worker starts for db2's subscription with a
valid oldest_xmin based on db2's horizon, in the next launcher cycle,
since db2 is already in the retained_dbids list, it will not fire the
reset again. The slot.xmin stays advanced, and when the db2 worker
reaches get_candidate_xid() with its old xmin, it hits the same
assertion.

TRAP: failed Assert("TransactionIdPrecedesOrEquals(MyLogicalRepWorker->oldest_nonremovable_xid,
oldest_running_xid)"), File: "worker.c", Line: 4536, PID: 60377
0 postgres 0x0000000100ac5144
ExceptionalCondition + 216
1 postgres 0x00000001007979c4
get_candidate_xid + 196
2 postgres 0x00000001007978ac
process_rdt_phase_transition + 128
3 postgres 0x00000001007973bc
maybe_advance_nonremovable_xid + 56
4 postgres 0x0000000100791b6c
LogicalRepApplyLoop + 1456
5 postgres 0x00000001007914f0 start_apply + 120
...

Attached is a TAP test that reproduces this issue using an injection
point. The patch applies on top of v2-0001.

--
Thanks,
Nisha

On Friday, September 4, 2026 3:27 PM Nisha Moond <nisha.moond412@gmail.com> wrote:

Thanks for the updated patch.

I was testing/reviewing the retention stop-resume path when two dbs (say
db1 and db2) are involved. I see a race condition between the launcher and
the worker that leads to the same assert.
When a sub resumes retention on db2, update_retention_status(true) sets
subretentionactive = true and calls ApplyLauncherWakeup() before the
worker restarts itself through apply_worker_exit().

Now if the launcher runs its next cycle while the old worker's entry is still
in_use(say we hold it just before apply_worker_exit), then in that cycle,
launcher will -
- add the db2 to retained_dbids.
- fire the reset because db2 is not yet in the set.
- Finds w != NULL (still old worker), so
compute_min_nonremovable_xid() reads the "InvalidTransactionId" left by
the earlier stop and returns without contributing to the result.

Now the situation is that update_conflict_slot_xmin() advances the slot to the
horizon of db1 (which can be way ahead of db2's horizon).
When eventually a new worker starts for db2's subscription with a valid
oldest_xmin based on db2's horizon, in the next launcher cycle, since db2 is
already in the retained_dbids list, it will not fire the reset again. The slot.xmin
stays advanced, and when the db2 worker reaches get_candidate_xid() with
its old xmin, it hits the same assertion.

TRAP: failed Assert("TransactionIdPrecedesOrEquals(MyLogicalRepWorker-
...

Attached is a TAP test that reproduces this issue using an injection point. The
patch applies on top of v2-0001.

Thanks for reporting this. I think we should skip updating the slot.xmin in the
cycle where the old worker hasn't stopped but retention has been resumed. Here's
the updated patch that fixes this. To make it cleaner, I also refactored the
logic slightly to centralize the decision of whether to update the slot.xmin.

On Thursday, September 3, 2026 5:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

4

With track_commit_timestamp off and retain_dead_tuples on,
FindDeletedTupleInLocalRel() returns false unconditionally and silently,

while

dead tuples are still retained.

This is not considered as a bug, as the documentation clearly states that
update_deleted requires track_commit_timestamp to be enabled, and we
emit a WARNING when retain_dead_tuples is enabled without it. If needed, we
could improve this by stopping retention when track_commit_timestamp is
disabled, but disabling track_commit_timestamp while retain_dead_tuples is enabled
seems like an extremely unlikely user behavior. So would be better to wait for user
feedback before taking any action.

5

max_retention_duration does not work for a disabled subscription or a
keep-failing worker, meaning retention cannot be stopped using this option
in those cases.

This is documented behavior (and user can disable the retain_dead_tuples
manually):

This option is effective only when retain_dead_tuples is enabled and the apply
worker associated with the subscription is active.

Both 4 and 5 are documented but will it be better to provide this
information explicitly, say via Logging when the conflcit_slot is not
getting advanced due to any of these reasons?

We can do that. It seems to me we could make this improvement for PG20 at this
stage. For PG19, I think we can improve the documentation to make it a bit
clearer instead. Amit and I discussed this off-list, and See 0002 for the patch
Amit shared to improve the doc.

Best Regards,
Zhijie Hou

On Fri, Sep 4, 2026 at 3:27 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

On Friday, September 4, 2026 3:27 PM Nisha Moond <nisha.moond412@gmail.com> wrote:

Thanks for the updated patch.

I was testing/reviewing the retention stop-resume path when two dbs (say
db1 and db2) are involved. I see a race condition between the launcher and
the worker that leads to the same assert.
When a sub resumes retention on db2, update_retention_status(true) sets
subretentionactive = true and calls ApplyLauncherWakeup() before the
worker restarts itself through apply_worker_exit().

Now if the launcher runs its next cycle while the old worker's entry is still
in_use(say we hold it just before apply_worker_exit), then in that cycle,
launcher will -
- add the db2 to retained_dbids.
- fire the reset because db2 is not yet in the set.
- Finds w != NULL (still old worker), so
compute_min_nonremovable_xid() reads the "InvalidTransactionId" left by
the earlier stop and returns without contributing to the result.

Now the situation is that update_conflict_slot_xmin() advances the slot to the
horizon of db1 (which can be way ahead of db2's horizon).
When eventually a new worker starts for db2's subscription with a valid
oldest_xmin based on db2's horizon, in the next launcher cycle, since db2 is
already in the retained_dbids list, it will not fire the reset again. The slot.xmin
stays advanced, and when the db2 worker reaches get_candidate_xid() with
its old xmin, it hits the same assertion.

TRAP: failed Assert("TransactionIdPrecedesOrEquals(MyLogicalRepWorker-
...

Attached is a TAP test that reproduces this issue using an injection point. The
patch applies on top of v2-0001.

Thanks for reporting this. I think we should skip updating the slot.xmin in the
cycle where the old worker hasn't stopped but retention has been resumed. Here's
the updated patch that fixes this. To make it cleaner, I also refactored the
logic slightly to centralize the decision of whether to update the slot.xmin.

On Thursday, September 3, 2026 5:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

4

With track_commit_timestamp off and retain_dead_tuples on,
FindDeletedTupleInLocalRel() returns false unconditionally and silently,

while

dead tuples are still retained.

This is not considered as a bug, as the documentation clearly states that
update_deleted requires track_commit_timestamp to be enabled, and we
emit a WARNING when retain_dead_tuples is enabled without it. If needed, we
could improve this by stopping retention when track_commit_timestamp is
disabled, but disabling track_commit_timestamp while retain_dead_tuples is enabled
seems like an extremely unlikely user behavior. So would be better to wait for user
feedback before taking any action.

5

max_retention_duration does not work for a disabled subscription or a
keep-failing worker, meaning retention cannot be stopped using this option
in those cases.

This is documented behavior (and user can disable the retain_dead_tuples
manually):

This option is effective only when retain_dead_tuples is enabled and the apply
worker associated with the subscription is active.

Both 4 and 5 are documented but will it be better to provide this
information explicitly, say via Logging when the conflcit_slot is not
getting advanced due to any of these reasons?

We can do that. It seems to me we could make this improvement for PG20 at this
stage. For PG19, I think we can improve the documentation to make it a bit
clearer instead. Amit and I discussed this off-list, and See 0002 for the patch
Amit shared to improve the doc.

I am still reviewing patch001; for patch 002 I have one comment. I
feel the following information should be added as a CAUTION or under
the existing WARNING section similar to how we explain the
data-acuumulation fact under CAUTION section for for
retain_dead_tuples GUC.

max_retention_duration
If the
+          subscription is disabled, or its apply worker is not running, the
+          retention duration is not evaluated and the information for conflict
+          detection continues to accumulate regardless of this setting. In that
+          case retention can only be stopped by disabling
+          <literal>retain_dead_tuples</literal>.
------------

Also, shall we change it slightly to:

Note that the retention duration is not evaluated while the
subscription is disabled or its apply worker is not running. Thus, the
information retained for conflict detection will continue to
accumulate regardless of this setting until the subscription is
enabled or its apply worker resumes. To prevent excessive
accumulation, consider disabling <literal>retain_dead_tuples</literal>
if the subscription will be inactive for an extended period.

thanks
Shveta

On Fri, Sep 4, 2026 at 3:27 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

Thanks for reporting this. I think we should skip updating the slot.xmin in the
cycle where the old worker hasn't stopped but retention has been resumed. Here's
the updated patch that fixes this. To make it cleaner, I also refactored the
logic slightly to centralize the decision of whether to update the slot.xmin.

I have made minor changes in the comments to make them clear. See attached.

Both 4 and 5 are documented but will it be better to provide this
information explicitly, say via Logging when the conflcit_slot is not
getting advanced due to any of these reasons?

We can do that. It seems to me we could make this improvement for PG20 at this
stage. For PG19, I think we can improve the documentation to make it a bit
clearer instead. Amit and I discussed this off-list, and See 0002 for the patch
Amit shared to improve the doc.

Thanks for sharing the doc updates. Can you please fold those in 0001
and share a combined patch?

--
With Regards,
Amit Kapila.

On Monday, September 7, 2026 2:37 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Sep 4, 2026 at 3:27 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
wrote:
I have made minor changes in the comments to make them clear. See
attached.

Both 4 and 5 are documented but will it be better to provide this
information explicitly, say via Logging when the conflcit_slot is
not getting advanced due to any of these reasons?

We can do that. It seems to me we could make this improvement for PG20
at this stage. For PG19, I think we can improve the documentation to
make it a bit clearer instead. Amit and I discussed this off-list, and
See 0002 for the patch Amit shared to improve the doc.

Thanks for sharing the doc updates. Can you please fold those in 0001 and
share a combined patch?

Thanks for the changes. I've merged them, here is the updated patch.

I confirmed that the patch applies cleanly on PG19 as well.

Best Regards,
Zhijie Hou

On Friday, September 4, 2026 8:03 PM shveta malik <shveta.malik@gmail.com> wrote:

I am still reviewing patch001; for patch 002 I have one comment. I feel the
following information should be added as a CAUTION or under the existing
WARNING section similar to how we explain the data-acuumulation fact
under CAUTION section for for retain_dead_tuples GUC.

max_retention_duration
If the
+          subscription is disabled, or its apply worker is not running, the
+          retention duration is not evaluated and the information for conflict
+          detection continues to accumulate regardless of this setting. In that
+          case retention can only be stopped by disabling
+          <literal>retain_dead_tuples</literal>.
------------

Also, shall we change it slightly to:

Note that the retention duration is not evaluated while the subscription is
disabled or its apply worker is not running. Thus, the information retained for
conflict detection will continue to accumulate regardless of this setting until
the subscription is enabled or its apply worker resumes. To prevent excessive
accumulation, consider disabling <literal>retain_dead_tuples</literal>
if the subscription will be inactive for an extended period.

Thanks for the comments.

I've added the suggested content to the CAUTION section in the new version of
the patch.

Best Regards,
Zhijie Hou

On Mon, Sep 7, 2026 at 1:23 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

Thanks for the changes. I've merged them, here is the updated patch.

I confirmed that the patch applies cleanly on PG19 as well.

Pushed.

--
With Regards,
Amit Kapila.

Dear Amit,

Thanks for pushing a patch. I found two BF failures due to the commit [1]https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bushmaster&amp;dt=2026-09-07%2019%3A48%3A18, [2]https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=canebrake&amp;dt=2026-09-07%2023%3A33%3A41.

In added test we tried to assign a transaction ID then ensure xmin of the slot
can be smaller. However, since query_until() could return before starting a new
transaction, there was a case that $next_xid cannot be computed correctly.
IIUC query_safe() returns after running all the command and it should be used here.
Also, I found another unnecessary usage of query_until(), it's also fixed.

Please see attached to fix the issue. This must be backpatched to PG19 and
attached could be applied.

[1]: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bushmaster&amp;dt=2026-09-07%2019%3A48%3A18
[2]: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=canebrake&amp;dt=2026-09-07%2023%3A33%3A41
[3]: ``` # Hold a transaction with an assigned transaction ID open in dbb, pinning its # oldest active transaction ID. my $dbb_session = $node_B->background_psql('dbb'); $dbb_session->query_until( qr/starting_bg_psql/, q{ \echo starting_bg_psql BEGIN; SELECT txid_current(); }); # Push the transaction ID counter clearly past the pinned transaction ID and # wait for the slot's xmin to advance past it. Only the apply worker in the # postgres database drives the slot's xmin here, and postgres has no old # transaction running. $next_xid = $node_B->safe_psql('postgres', "SELECT txid_current() + 1"); ... # The slot's xmin must regress to the horizon pinned in dbb. ok( $node_B->poll_query_until( 'postgres', "SELECT xmin::text::bigint < $next_xid FROM pg_replication_slots WHERE slot_name = 'pg_conflict_detection'" ), "slot xmin regressed to the horizon pinned in dbb"); ``` Best regards, Hayato Kuroda FUJITSU LIMITED
```
# Hold a transaction with an assigned transaction ID open in dbb, pinning its
# oldest active transaction ID.
my $dbb_session = $node_B->background_psql('dbb');
$dbb_session->query_until(
qr/starting_bg_psql/, q{
\echo starting_bg_psql
BEGIN;
SELECT txid_current();
});
# Push the transaction ID counter clearly past the pinned transaction ID and
# wait for the slot's xmin to advance past it. Only the apply worker in the
# postgres database drives the slot's xmin here, and postgres has no old
# transaction running.
$next_xid = $node_B->safe_psql('postgres', "SELECT txid_current() + 1");
...
# The slot's xmin must regress to the horizon pinned in dbb.
ok( $node_B->poll_query_until(
'postgres',
"SELECT xmin::text::bigint < $next_xid FROM pg_replication_slots WHERE slot_name = 'pg_conflict_detection'"
),
"slot xmin regressed to the horizon pinned in dbb");
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Tue, Sep 8, 2026 at 9:52 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

In added test we tried to assign a transaction ID then ensure xmin of the slot
can be smaller. However, since query_until() could return before starting a new
transaction, there was a case that $next_xid cannot be computed correctly.
IIUC query_safe() returns after running all the command and it should be used here.
Also, I found another unnecessary usage of query_until(), it's also fixed.

Please see attached to fix the issue. This must be backpatched to PG19 and
attached could be applied.

Thanks for the analysis and patch, it looked good to me. Pushed.

--
With Regards,
Amit Kapila.