Per-table resync for logical replication subscriptions

here is the WIP patch for the disabled-subscription case.
It adds:

ALTER SUBSCRIPTION name REFRESH TABLE table_name
[ WITH (copy_data = true, truncate = true) ]

The command is subscriber-local: it resets just the named relation in
pg_subscription_rel back to the initial sync state (optionally
truncating the local copy first), so a tablesync worker re-copies that
one table on the next enable, reusing the existing tablesync machinery.
Publication membership and other tables are left untouched. As
discussed, this first version requires the subscription to be disabled.

The tests are included in the same patch: a TAP test at
src/test/subscription/t/039_refresh_table.pl that

- resyncs one drifted table on a two-node setup and confirms it
matches the publisher again after re-enabling;
- confirms only the targeted relation is reset (a sibling table is
left in 'r' state);
- checks the rejection cases: enabled subscription, table not in the
subscription, a sequence, truncate without copy_data, and running
inside a transaction block.

The full src/test/subscription suite and the core regression tests pass
with the patch applied.

Open questions if this direction makes sense ,I would still like feedback
on: the command spelling
(REFRESH TABLE vs RESYNC TABLE), and whether the enabled-subscription
case is worth doing as a follow-up (it needs the running apply worker to
notice and re-read the reset relation state).

Regards,
Cagri Biroglu

On Tue, Jul 7, 2026 at 11:41 AM Cagri Biroglu <cagri.biroglu@adyen.com>
wrote:

Show quoted text

Hi hackers,

I would like to propose a subscriber-side way to re-copy a single table
that is already part of a logical replication subscription.

At the moment, I do not think PostgreSQL has a supported way to do this
for one table on one subscriber. ALTER SUBSCRIPTION ... REFRESH
PUBLICATION only handles tables that were added to the subscribed
publications since the subscription was created, or since the last refresh.
It does not re-copy tables that are already known to the subscription.

That is fine for the normal “new table was added to the publication” case,
but it leaves a gap when only one subscriber needs one table to be
re-seeded. Some examples are:

-

the subscriber table became inconsistent;
-

a row filter changed and the subscriber needs a fresh copy;
-

a forwarder/subscriber needs to repair one table without disturbing
other subscribers.

The usual workaround is to temporarily change publication membership,
refresh the subscription, truncate the local table, add the table back, and
refresh again. That works in simple topologies, but it is not really local
to the subscriber. In a fan-out topology it can affect sibling subscribers,
and in a forwarding topology the truncate can be replicated further
downstream if truncates are published.

There is also the unsupported option of updating pg_subscription_rel by
hand to put the table back into the initial sync state. That is not safe
either, because it races with the apply worker and does not handle the
surrounding cleanup and invalidation work.

There is already a similar mechanism for sequences. ALTER SUBSCRIPTION
... REFRESH SEQUENCES can re-synchronize sequence data for sequences
already known to the subscription, while REFRESH PUBLICATION only
discovers newly added ones. This proposal is essentially the table-side
equivalent of that idea.

What I am proposing is something like:

ALTER SUBSCRIPTION name REFRESH TABLE table_name
[ WITH (copy_data = true, truncate = true) ];

or, if people prefer the wording:

ALTER SUBSCRIPTION name RESYNC TABLE table_name
[ WITH (copy_data = true, truncate = true) ];

The command would be subscriber-local and would not change publication
membership. Internally, it would reuse the existing table synchronization
machinery by resetting the selected relation back to the initial sync
state, so that a tablesync worker copies it again and moves it through the
normal i -> d -> s -> r states.

I tested the current behavior on current devel with a small three-node
setup:

-

upstream publisher U;
-

forwarding subscriber/publisher F;
-

downstream subscriber D;
-

sibling subscriber S.

The test script keeps a writer running and prints per-table checksums at
each step.

The important observations were:

1.

ALTER SUBSCRIPTION ... REFRESH PUBLICATION on F is a no-op for an
already subscribed table. The table remained in ready state, the tuple
metadata did not change, no WAL was generated, and no sync worker was
started.
2.

The publication-membership workaround is not local. In the forwarding
case, truncating the table on F also wiped the table on D. In the
fan-out case, the sibling subscriber S was left behind because the
publication changes were global but the repair was only intended for F.
3.

A controlled subscriber-local reset of only the selected table on F
produced matching checksums again without touching the publication or S.
Doing the same thing as a bare catalog update while the subscription
remained enabled was not reliable; the apply worker could continue with
cached state and the table diverged.

So I do not think the hard part is inventing a new synchronization
mechanism. The existing tablesync path already does most of the work. The
missing piece is a supported command that performs the reset safely:
stopping or coordinating with workers, cleaning up per-table sync state and
slots/origins where needed, optionally truncating the target table,
invalidating caches, taking the right locks, and validating that the
requested table is actually part of the subscription.

For a first version, I think it would be reasonable to require the
subscription to be disabled. That avoids the apply-worker race and keeps
the patch smaller. A later version could support the enabled case, probably
by making the running worker notice and re-read the changed relation state.

I have a WIP patch for the disabled-subscription, executor changes, and a
TAP test. Before polishing it further, I would like to get feedback on
whether this direction makes sense ?

Regards,

Cagri Biroglu

On Wed, Jul 8, 2026 at 8:55 PM Cagri Biroglu <cagri.biroglu@adyen.com> wrote:

here is the WIP patch for the disabled-subscription case.
It adds:

ALTER SUBSCRIPTION name REFRESH TABLE table_name
[ WITH (copy_data = true, truncate = true) ]

...

Open questions if this direction makes sense ,I would still like feedback on: the command spelling
(REFRESH TABLE vs RESYNC TABLE), and whether the enabled-subscription
case is worth doing as a follow-up (it needs the running apply worker to
notice and re-read the reset relation state).

(My feedback only about the syntax question)

As you said in the OP, this proposal is a bit like a table equivalent
of "ALTER SUBSCRIPTION sub REFRESH SEQUENCES".

IIRC we went through similar syntax debates when that command was
being developed.

Some alternatives were proposed [1]/messages/by-id/CALDaNm30iA_N4Dfeyp__YdLnbm9oatw_+VyCfNfbQmZiS_ybCw@mail.gmail.com
ALTER SUBSCRIPTION sub RESYNC PUBLICATION SEQUENCES;
ALTER SUBSCRIPTION sub RESYNC SEQUENCES;

And, then more [2]/messages/by-id/CAA4eK1JJUv2Kykb=JvdTzq+qDzkbD3m+YPAF6qz6Sm-qJ+wF8g@mail.gmail.com
ALTER SUBSCRIPTION sub REPLICATE PUBLICATION SEQUENCES;
ALTER SUBSCRIPTION sub RESYNC PPUBLICATION SEQUENCES;
ALTER SUBSCRIPTION sub SYNC PUBLICATION SEQUENCES;
ALTER SUBSCRIPTION sub MERGE PUBLICATION SEQUENCES;

It went back and forth for a while...

Finally, I proposed [3]/messages/by-id/CAHut+PvOmCe2fO4LPwJtROierhGLtHY+++r9PXF8=LLQ2m=AXw@mail.gmail.com and gave some reasons for just having:
ALTER SUBSCRIPTION sub REFRESH SEQUENCES;

And that's where it ended up...

~~~

If your proposal is to proceed, then IMO it would be better to
continue the same pattern, avoiding introducing new keywords unless
absolutely necessary.

So, your v1 patch syntax "REFRESH TABLE" looked good to me. A similar
reasoning to "REFRESH SEQUENCES" [3]/messages/by-id/CAHut+PvOmCe2fO4LPwJtROierhGLtHY+++r9PXF8=LLQ2m=AXw@mail.gmail.com can be used to explain it.

======
[1]: /messages/by-id/CALDaNm30iA_N4Dfeyp__YdLnbm9oatw_+VyCfNfbQmZiS_ybCw@mail.gmail.com
[2]: /messages/by-id/CAA4eK1JJUv2Kykb=JvdTzq+qDzkbD3m+YPAF6qz6Sm-qJ+wF8g@mail.gmail.com
[3]: /messages/by-id/CAHut+PvOmCe2fO4LPwJtROierhGLtHY+++r9PXF8=LLQ2m=AXw@mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

Hi,

On Wed, Jul 8, 2026 at 3:55 AM Cagri Biroglu <cagri.biroglu@adyen.com> wrote:

here is the WIP patch for the disabled-subscription case.
It adds:

ALTER SUBSCRIPTION name REFRESH TABLE table_name
[ WITH (copy_data = true, truncate = true) ]

The command is subscriber-local: it resets just the named relation in
pg_subscription_rel back to the initial sync state (optionally
truncating the local copy first), so a tablesync worker re-copies that
one table on the next enable, reusing the existing tablesync machinery.
Publication membership and other tables are left untouched. As
discussed, this first version requires the subscription to be disabled.

The tests are included in the same patch: a TAP test at
src/test/subscription/t/039_refresh_table.pl that

- resyncs one drifted table on a two-node setup and confirms it
matches the publisher again after re-enabling;
- confirms only the targeted relation is reset (a sibling table is
left in 'r' state);
- checks the rejection cases: enabled subscription, table not in the
subscription, a sequence, truncate without copy_data, and running
inside a transaction block.

The full src/test/subscription suite and the core regression tests pass
with the patch applied.

Open questions if this direction makes sense ,I would still like feedback on: the command spelling
(REFRESH TABLE vs RESYNC TABLE), and whether the enabled-subscription
case is worth doing as a follow-up (it needs the running apply worker to
notice and re-read the reset relation state).

REFRESH TABLE is better to me. It's more consistent with the existing
commands such as REFRESH REQUENCES.

I've not looked at the patch in depth yet but I have a few comments:

I think it might be a good idea that the REFRESH TABLE command can
take multiple tables rather than one table.

---
Why does the REFRESH TABLE command require for the subscription to be
disabled while REFRESH PUBLICATIONS doesn't?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Thanks a lot for looking at this.

Before the point-by-point: one change after getting feedback on this

from the sketch in my first mail

is that I dropped the WITH (copy_data, truncate) options. For refreshing

a table that is already subscribed, only one combination is useful, so
REFRESH TABLE now always truncates the local copy and re-copies (what
would have been copy_data = true, truncate = true). Without truncating,
the re-copy hits duplicate-key errors or doubles the rows; skipping the
copy leaves the drift in place. So neither other combination is
meaningful, and there is nothing to configure.

REFRESH TABLE is better to me. It's more consistent with the existing
commands such as REFRESH SEQUENCES.

Great, I've kept REFRESH TABLE.

I think it might be a good idea that the REFRESH TABLE command can
take multiple tables rather than one table.

Agreed. v2 makes REFRESH TABLE take a comma-separated list:

ALTER SUBSCRIPTION name REFRESH TABLE t1, t2, ...

It's all-or-nothing: every named relation is validated (part of the
subscription, not a sequence, exists) before any of them is reset, so a
bad table name aborts the whole command without touching the others. The
listed tables are also truncated together, which lets a set connected by
foreign keys be re-seeded as a unit.

One note on scope: this doesn't add resource risk over doing one table
at a time, because the re-copy is still performed by tablesync workers
that are capped by max_sync_workers_per_subscription; resetting N tables
just queues them rather than starting N copies at once.

Why does the REFRESH TABLE command require for the subscription to be
disabled while REFRESH PUBLICATIONS doesn't?

Good question, but I'd actually argue the disabled requirement is a
reasonable design choice here rather than a limitation to remove.

The reason the two commands differ is that REFRESH PUBLICATION only adds
*new* relations to pg_subscription_rel (in init state), whereas REFRESH
TABLE resets a relation that is already in ready state, and those hit
different caches in the apply worker:

- The set of not-ready relations is cached (table_states_not_ready in
syncutils.c) and correctly invalidated by the SUBSCRIPTIONRELMAP
syscache callback (InvalidateSyncingRelStates). So after the reset the
apply worker does notice the table is non-ready and does launch a
tablesync worker for it. That part works whether enabled or not.

- However, whether the apply worker applies incoming changes for a
relation is decided by should_apply_changes_for_rel(), which reads the
per-relation LogicalRepRelMapEntry.state. That state is only refreshed
from the catalog while it is not READY:

/* relation.c, logicalrep_rel_open() */
if (entry->state != SUBREL_STATE_READY)
entry->state = GetSubscriptionRelState(...);

For an already-ready table the entry stays READY, and the relcache
invalidation only clears localrelvalid (rebuilds the attrmap); it does
not reset entry->state. So the running apply worker keeps treating the
table as READY and keeps applying live changes to it, at the same time
as the freshly launched tablesync worker re-copies it, with no handoff
on the sync LSN. In my testing that reliably diverges the table.

For a newly added table (REFRESH PUBLICATION) there is no stale READY
entry, so the normal init -> datasync -> syncdone -> ready handoff works
without any disable.

Requiring the subscription to be disabled sidesteps this cleanly. When
you re-enable, a fresh apply worker starts and reads the reset table's
state from the catalog as init, so the same init -> datasync -> syncdone
-> ready handoff kicks in, exactly as for a newly added table. And the
disable only needs to bracket the quick catalog reset, not the re-copy
itself: REFRESH TABLE just truncates and flips the state, and after
ENABLE the tablesync worker does the (possibly long) copy while the
apply worker resumes the other tables as usual, so the pause is short
and scoped to the metadata change, not the data copy.

Supporting the enabled case would instead mean reaching into the running
apply worker to force it to drop and re-read the cached state of the
reset relation mid-flight. That is doable, but it adds moving parts and
a new invalidation path to get right, for what is a deliberate,
occasional maintenance action where briefly disabling the subscription
does no harm. So my inclination is to keep the disabled requirement here
rather than trade that simplicity away. Happy to reconsider if you think
the enabled case matters enough to justify the extra machinery or i am
missing sth.

Regards,

Show quoted text

Cagri Biroglu

I wanted to follow up on this patch, as there has not been any activity on
the thread for some time.

The proposal is still relevant from my side, and I am happy to rebase the
patch, run additional tests, or address any design concerns or review
feedback.

On Fri, 10 Jul 2026 at 12:38, Cagri Biroglu <cagri.biroglu@adyen.com> wrote:

Show quoted text

Thanks a lot for looking at this.

Before the point-by-point: one change after getting feedback on this

from the sketch in my first mail

is that I dropped the WITH (copy_data, truncate) options. For refreshing

a table that is already subscribed, only one combination is useful, so
REFRESH TABLE now always truncates the local copy and re-copies (what
would have been copy_data = true, truncate = true). Without truncating,
the re-copy hits duplicate-key errors or doubles the rows; skipping the
copy leaves the drift in place. So neither other combination is
meaningful, and there is nothing to configure.

REFRESH TABLE is better to me. It's more consistent with the existing
commands such as REFRESH SEQUENCES.

Great, I've kept REFRESH TABLE.

I think it might be a good idea that the REFRESH TABLE command can
take multiple tables rather than one table.

Agreed. v2 makes REFRESH TABLE take a comma-separated list:

ALTER SUBSCRIPTION name REFRESH TABLE t1, t2, ...

It's all-or-nothing: every named relation is validated (part of the
subscription, not a sequence, exists) before any of them is reset, so a
bad table name aborts the whole command without touching the others. The
listed tables are also truncated together, which lets a set connected by
foreign keys be re-seeded as a unit.

One note on scope: this doesn't add resource risk over doing one table
at a time, because the re-copy is still performed by tablesync workers
that are capped by max_sync_workers_per_subscription; resetting N tables
just queues them rather than starting N copies at once.

Why does the REFRESH TABLE command require for the subscription to be
disabled while REFRESH PUBLICATIONS doesn't?

Good question, but I'd actually argue the disabled requirement is a
reasonable design choice here rather than a limitation to remove.

The reason the two commands differ is that REFRESH PUBLICATION only adds
*new* relations to pg_subscription_rel (in init state), whereas REFRESH
TABLE resets a relation that is already in ready state, and those hit
different caches in the apply worker:

- The set of not-ready relations is cached (table_states_not_ready in
syncutils.c) and correctly invalidated by the SUBSCRIPTIONRELMAP
syscache callback (InvalidateSyncingRelStates). So after the reset the
apply worker does notice the table is non-ready and does launch a
tablesync worker for it. That part works whether enabled or not.

- However, whether the apply worker applies incoming changes for a
relation is decided by should_apply_changes_for_rel(), which reads the
per-relation LogicalRepRelMapEntry.state. That state is only refreshed
from the catalog while it is not READY:

/* relation.c, logicalrep_rel_open() */
if (entry->state != SUBREL_STATE_READY)
entry->state = GetSubscriptionRelState(...);

For an already-ready table the entry stays READY, and the relcache
invalidation only clears localrelvalid (rebuilds the attrmap); it does
not reset entry->state. So the running apply worker keeps treating the
table as READY and keeps applying live changes to it, at the same time
as the freshly launched tablesync worker re-copies it, with no handoff
on the sync LSN. In my testing that reliably diverges the table.

For a newly added table (REFRESH PUBLICATION) there is no stale READY
entry, so the normal init -> datasync -> syncdone -> ready handoff works
without any disable.

Requiring the subscription to be disabled sidesteps this cleanly. When
you re-enable, a fresh apply worker starts and reads the reset table's
state from the catalog as init, so the same init -> datasync -> syncdone
-> ready handoff kicks in, exactly as for a newly added table. And the
disable only needs to bracket the quick catalog reset, not the re-copy
itself: REFRESH TABLE just truncates and flips the state, and after
ENABLE the tablesync worker does the (possibly long) copy while the
apply worker resumes the other tables as usual, so the pause is short
and scoped to the metadata change, not the data copy.

Supporting the enabled case would instead mean reaching into the running
apply worker to force it to drop and re-read the cached state of the
reset relation mid-flight. That is doable, but it adds moving parts and
a new invalidation path to get right, for what is a deliberate,
occasional maintenance action where briefly disabling the subscription
does no harm. So my inclination is to keep the disabled requirement here
rather than trade that simplicity away. Happy to reconsider if you think
the enabled case matters enough to justify the extra machinery or i am
missing sth.

Regards,

Cagri Biroglu

On Fri, Jul 31, 2026 at 12:00 AM Cagri Biroglu <cagri.biroglu@adyen.com> wrote:

I wanted to follow up on this patch, as there has not been any activity on the thread for some time.

The proposal is still relevant from my side, and I am happy to rebase the patch, run additional tests, or address any design concerns or review feedback.

Thank you for submitting the patch!

I'll review the patch and share my comments. Meanwhile, please
register this patch to the next CommitFest[1]https://commitfest.postgresql.org/61/ if not yet.

Regards,

[1]: https://commitfest.postgresql.org/61/

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Hi again,

Thanks, and thanks for picking this up. I've registered it in the open
CommitFest (PG20-2).

Attached is v3, which is v2 rebased onto current master. Master removed
Subscription.conninfo, so the one code change is that the mid-sync path
now resolves the connection string with SubscriptionConninfo(sub). It
does that at the point of use rather than up front, because the common
case here is a relation in ready state, which needs no publisher
connection at all. Nothing else changed, so your review of v2 still
applies.

Regards,
Cagri Biroglu

On Sat, Aug 8, 2026 at 2:49 AM Masahiko Sawada <sawada.mshk@gmail.com>
wrote:

Show quoted text

On Fri, Jul 31, 2026 at 12:00 AM Cagri Biroglu <cagri.biroglu@adyen.com>
wrote:

I wanted to follow up on this patch, as there has not been any activity

on the thread for some time.

The proposal is still relevant from my side, and I am happy to rebase

the patch, run additional tests, or address any design concerns or review
feedback.

Thank you for submitting the patch!

I'll review the patch and share my comments. Meanwhile, please
register this patch to the next CommitFest[1] if not yet.

Regards,

[1] https://commitfest.postgresql.org/61/

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Mon, Aug 10, 2026 at 1:53 AM Cagri Biroglu <cagri.biroglu@adyen.com> wrote:

Hi again,

Thanks, and thanks for picking this up. I've registered it in the open
CommitFest (PG20-2).

+1

Attached is v3, which is v2 rebased onto current master. Master removed
Subscription.conninfo, so the one code change is that the mid-sync path
now resolves the connection string with SubscriptionConninfo(sub). It
does that at the point of use rather than up front, because the common
case here is a relation in ready state, which needs no publisher
connection at all. Nothing else changed, so your review of v2 still
applies.

Thank you for updating the patch! I've reviewed the v3 patch and here
are review comments:

---
+           PG_TRY();
+           {
+               ReplicationSlotNameForTablesync(sub->oid, relid, syncslotname,
+                                               sizeof(syncslotname));
+               ReplicationSlotDropAtPubNode(wrconn, syncslotname, true);
+           }
+           PG_FINALLY();
+           {
+               walrcv_disconnect(wrconn);
+           }
+           PG_END_TRY();

I think it's better to drop the replication slots at the very end of
AlterSubscription_refresh_table() if possible because
ExecuteTruncate() can fail for many reasons (e.g., foreign key
constraints, insufficient privileges etc.).

Or is it worth considering an alternative design that the tablesync
worker truncates the table in the same transaction of the COPY?

---
Since REFRESH TABLE always copies the table data, I think we need the
following twophasestate check that is done in REFRESH PUBLICATION
command:

if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED &&
opts.copy_data)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("ALTER SUBSCRIPTION ... REFRESH PUBLICATION with
copy_data is not allowed when two_phase is enabled"),
errhint("Use ALTER SUBSCRIPTION ... REFRESH PUBLICATION
with copy_data = false, or use DROP/CREATE SUBSCRIPTION.")));

---
+
+       relid = RangeVarGetRelid(rv, AccessShareLock, false);
+

I think we should take an AccessExclusiveLock when opening the table
instead of escalating the lock level during the truncation.

---
+   tstmt = makeNode(TruncateStmt);
+   tstmt->relations = truncrels;
+   tstmt->restart_seqs = false;
+   tstmt->behavior = DROP_RESTRICT;
+   ExecuteTruncate(tstmt);

Since we already resolve individual specified table names we should
use their OID rather than passing a list of RangeVar to let
ExecuteTransaction() resolve the OIDs again. So we should use
ExecuteTruncateGuts() instead.

---
+               if (sub->enabled)
+                   ereport(ERROR,
+                           errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                           errmsg("%s is not allowed for enabled
subscriptions",
+                                  "ALTER SUBSCRIPTION ... REFRESH TABLE"),
+                           errhint("Disable the subscription with
ALTER SUBSCRIPTION ... DISABLE first."));

I think we need to check if the apply worker actually stopped, in
addition to this check. Sub->enabled being false doesn't guarantee
that the apply worker is not working.

---
+   foreach_oid(relid, relids)
+   {
...
+       if (relstate != SUBREL_STATE_READY && relstate != SUBREL_STATE_SYNCDONE)
+       {
...
+
+           must_use_password = sub->passwordrequired && !sub->ownersuperuser;
+           wrconn = walrcv_connect(SubscriptionConninfo(sub), true, true,
+                                   must_use_password, sub->name, &err);

The function establishes connections for each relation. We should use
the one connection for all relations.

---
Please update psql's tab-completion for the new syntax.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

At 2026-08-10 16:53:04, "Cagri Biroglu" <cagri.biroglu@adyen.com> wrote:

Hi again,
Thanks, and thanks for picking this up. I've registered it in the open
CommitFest (PG20-2).
Attached is v3, which is v2 rebased onto current master. Master removed
Subscription.conninfo, so the one code change is that the mid-sync path
now resolves the connection string with SubscriptionConninfo(sub). It
does that at the point of use rather than up front, because the common
case here is a relation in ready state, which needs no publisher
connection at all. Nothing else changed, so your review of v2 still
applies.

hi,
I have some review feedback on the v3-0001 patch.
The handling of inheritance tables and partition child tables with REFRESH TABLE seems incomplete; its current behavior is equivalent to the TRUNCATE ONLY syntax.
This could result in the following problems:
After executing REFRESH TABLE and before the tablesync worker completes synchronization, running SELECT * FROM parent will only return residual data from child tables, while rows residing directly in the parent table will be missing.
The partitioned parent table cannot be refreshed directly.
After the subscription is disabled, the states of child tables are not reset.
I originally expected data conflicts would arise, yet testing showed no critical data anomalies, and the data eventually became consistent.
Could you please assess whether this constitutes an issue that needs addressing?
Besides, I began reviewing this patch yesterday but have progressed rather slowly. The patch is now tagged "Waiting on Author". I apologize if this has held up your development work.

Best regards,

--

Yilin Zhang

Dear Cagri,

Thanks for updating the patch. I read and tested your patch. Here are my comments.

01.
According to the REFRESH PUBLICATION command, we must prohibit to synchronize
tables if the two_phase is enabled, but REFRESH TABLE seems to bypass the restriction.
Also, if there are prepared transactions done by the subscription and they modify
target relations, the command would stuck forever - the apply worker won't start
again. So should we have the same guard as the REFRESH PUBLICATION?

02.
You have already checked the case the existence of tablesync workers, but
not for the leader worker. I feel we should ensure via logicalrep_workers_find()
like ALTER SUBSCRIPTION SET (two_phase). Also, we can remove the part from
AlterSubscription_refresh_table().

03.
Regarding the partition table, I found the case that TRUNCATE happened but
srsubstate cannot be updated. It's because ExecuteTruncate() truncates tables all
child tables but AlterSubscription_refresh_table() updates tuples relid is
exactly matched. Per my experiment, this can cause the issue if the different publications
publish the root and child separately, and they are subscribed by the different subscription.
See attached reproducer.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi Masahiko,

Thank you for the review. Attached is v4.

Taking the design question first, since it is the one that could have
changed the shape of the patch.

Or is it worth considering an alternative design that the tablesync
worker truncates the table in the same transaction of the COPY?

I looked into this and I do not think it works, for three reasons. I would
be glad to be told if I have missed something.

First, it would make any table that is the target of a foreign key
impossible to resync. heap_truncate_check_FKs() rejects on the existence
of the constraint, not on whether the referencing table has rows.

A tablesync worker is responsible for exactly one relation, so it can only
ever issue a single-relation truncate, which for such a table always
fails. Truncating all of the named relations in one statement is also what
makes the multi-table form you asked for in the first review work for a
set connected by foreign keys.

Second, the lock would span the whole COPY, and it would stall the
subscription.

The first half follows from TRUNCATE taking ACCESS EXCLUSIVE and from
"Once acquired, a lock is normally held until the end of the transaction"
, so the lock is not released when the TRUNCATE statement
finishes. Measured, with a 30 million row COPY after the TRUNCATE in one
transaction.

The second half I also tested. Holding
AccessExclusiveLock on one relation of a subscription (via a prepared
transaction, so no session is involved) while the publisher writes to that
relation and to a second, unlocked one:

elapsed rows in 'b' apply worker waiting on ungranted lock on 'a'
1s 0 Lock:relation RowExclusiveLock
...
6s 0 Lock:relation RowExclusiveLock
after ROLLBACK PREPARED: a=1 b=1

Table b is never locked, yet it receives nothing for as long as the lock is
held, because apply is single-threaded per subscription. The reason it
blocks even though the relation is still syncing and its changes would be
skipped is that apply_handle_insert() locks first and decides afterwards:

rel = logicalrep_rel_open(relid, RowExclusiveLock);
if (!should_apply_changes_for_rel(rel))

This is also the choice LogicalRepSyncTableStart() documents:

/*
* Use a standard write lock here. It might be better to disallow access
* to the table while it's being synchronized. But we don't want to
block
* the main apply process from working and it has to open the relation
in
* RowExclusiveLock when remapping remote relation id to local one.
*/
rel = table_open(MyLogicalRepWorker->relid, RowExclusiveLock);

Third, truncate failures would move from the user's session into a
background worker. Today a foreign key problem, a missing privilege or a
lock timeout is reported to the client and nothing has changed; in the
alternative they would surface inside a worker that fails and retries.

There is also a smaller structural cost as far as i could see: srsubstate
'i'
currently means "new relation, never copied", and it would have to also
mean
"existing relation, truncate before copying". Those cannot be conflated,
because a
user may deliberately preload a table before adding it to a subscription,
so a new state or column would be needed.

I think it's better to drop the replication slots at the very end of
AlterSubscription_refresh_table()

Done. The slots are dropped after the truncate and after the state reset,
as the last thing the function does, since dropping a remote slot is not
transactional and everything above it can still fail for ordinary reasons.
The relations needing a slot dropped are collected in the first pass,
which made your sixth point below fall out naturally.

Since REFRESH TABLE always copies the table data, I think we need the
following twophasestate check

Added. There is no copy_data = false escape here, since re-copying is the
entire purpose of the command, so the wording differs slightly from
REFRESH PUBLICATION:

ERROR: ALTER SUBSCRIPTION ... REFRESH TABLE is not allowed when
two_phase is enabled
HINT: Use ALTER SUBSCRIPTION ... SET (two_phase = false), or use
DROP/CREATE SUBSCRIPTION.

I think we should take an AccessExclusiveLock when opening the table
instead of escalating the lock level during the truncation.

Done, the relation is locked at that level in the resolution pass.

we should use their OID rather than passing a list of RangeVar ... So we
should use ExecuteTruncateGuts() instead.

Done, and two things came out of it worth flagging.

ExecuteTruncateGuts() performs no privilege check on the tables; the check
in the ExecuteTruncate() path comes from RangeVarCallbackForTruncate(), so
calling the guts directly would have silently dropped it. The command now
requires TRUNCATE on each named relation explicitly, with tests for the
denied and the granted case.

The other is partitioned tables. Since ExecuteTruncateGuts() truncates
exactly what it is handed, the partitions of a named partitioned table
have to be collected too, which the patch does the same way
apply_handle_truncate() does, skipping other backends' temp tables.

I think we need to check if the apply worker actually stopped, in
addition to this check. Sub->enabled being false doesn't guarantee that
the apply worker is not working.

Agreed, and this one grew. Checking the apply worker alone is not enough,
because a tablesync worker outlives its apply worker. Measured on a 1 GB,
4 million row table, disabling the subscription mid-copy:

elapsed apply workers tablesync workers rows committed
0s 0 1 0
...
9s 0 1 0
10s 0 0 4000000

The apply worker is gone immediately, while the tablesync worker runs for
another ten seconds and then commits the whole copy. Any check that looked
only at WORKERTYPE_APPLY would have passed at 0s, with a worker still
holding the relation and about to write four million rows into it. v4 waits
for all worker types instead:

if (logicalrep_workers_find(subid, false, true))
ereport(ERROR, ...
errmsg("cannot %s when logical replication worker is still
running", ...

only_running is false, as DROP SUBSCRIPTION does for the same reason: a
worker that is in_use but has not attached yet is the dangerous one, since
it is about to read the relation state we are rewriting. I checked this
cannot lock a user out permanently, as WaitForReplicationWorkerAttach()
either observes the attach or calls logicalrep_worker_cleanup().

On your point, using one connection to the publisher for all of the
relations: done, as noted above. A single walrcv_connect() serves every
slot, and the common case where all named relations are ready needs no
publisher connection at all.

Please update psql's tab-completion for the new syntax.

Done, including completion of table names after the keyword and after a
comma, verified by hand against a readline build.

Not something was raised, but v3 had no documentation at all. v4 adds a
REFRESH TABLE entry to alter_subscription.sgml covering the requirements,
the all-or-nothing behaviour of the list, and the partition, inheritance
and shared-relation behaviour.

Regards,
Cagri Biroglu

On Mon, Aug 10, 2026 at 11:30 PM Masahiko Sawada <sawada.mshk@gmail.com>
wrote:

Show quoted text

On Mon, Aug 10, 2026 at 1:53 AM Cagri Biroglu <cagri.biroglu@adyen.com>
wrote:

Hi again,

Thanks, and thanks for picking this up. I've registered it in the open
CommitFest (PG20-2).

+1

Attached is v3, which is v2 rebased onto current master. Master removed
Subscription.conninfo, so the one code change is that the mid-sync path
now resolves the connection string with SubscriptionConninfo(sub). It
does that at the point of use rather than up front, because the common
case here is a relation in ready state, which needs no publisher
connection at all. Nothing else changed, so your review of v2 still
applies.

Thank you for updating the patch! I've reviewed the v3 patch and here
are review comments:

---
+           PG_TRY();
+           {
+               ReplicationSlotNameForTablesync(sub->oid, relid,
syncslotname,
+                                               sizeof(syncslotname));
+               ReplicationSlotDropAtPubNode(wrconn, syncslotname, true);
+           }
+           PG_FINALLY();
+           {
+               walrcv_disconnect(wrconn);
+           }
+           PG_END_TRY();

I think it's better to drop the replication slots at the very end of
AlterSubscription_refresh_table() if possible because
ExecuteTruncate() can fail for many reasons (e.g., foreign key
constraints, insufficient privileges etc.).

Or is it worth considering an alternative design that the tablesync
worker truncates the table in the same transaction of the COPY?

---
Since REFRESH TABLE always copies the table data, I think we need the
following twophasestate check that is done in REFRESH PUBLICATION
command:

if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED &&
opts.copy_data)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("ALTER SUBSCRIPTION ... REFRESH PUBLICATION with
copy_data is not allowed when two_phase is enabled"),
errhint("Use ALTER SUBSCRIPTION ... REFRESH PUBLICATION
with copy_data = false, or use DROP/CREATE SUBSCRIPTION.")));

---
+
+       relid = RangeVarGetRelid(rv, AccessShareLock, false);
+

I think we should take an AccessExclusiveLock when opening the table
instead of escalating the lock level during the truncation.

---
+   tstmt = makeNode(TruncateStmt);
+   tstmt->relations = truncrels;
+   tstmt->restart_seqs = false;
+   tstmt->behavior = DROP_RESTRICT;
+   ExecuteTruncate(tstmt);

Since we already resolve individual specified table names we should
use their OID rather than passing a list of RangeVar to let
ExecuteTransaction() resolve the OIDs again. So we should use
ExecuteTruncateGuts() instead.

---
+               if (sub->enabled)
+                   ereport(ERROR,
+
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                           errmsg("%s is not allowed for enabled
subscriptions",
+                                  "ALTER SUBSCRIPTION ... REFRESH TABLE"),
+                           errhint("Disable the subscription with
ALTER SUBSCRIPTION ... DISABLE first."));

I think we need to check if the apply worker actually stopped, in
addition to this check. Sub->enabled being false doesn't guarantee
that the apply worker is not working.

---
+   foreach_oid(relid, relids)
+   {
...
+       if (relstate != SUBREL_STATE_READY && relstate !=
SUBREL_STATE_SYNCDONE)
+       {
...
+
+           must_use_password = sub->passwordrequired &&
!sub->ownersuperuser;
+           wrconn = walrcv_connect(SubscriptionConninfo(sub), true, true,
+                                   must_use_password, sub->name, &err);

The function establishes connections for each relation. We should use
the one connection for all relations.

---
Please update psql's tab-completion for the new syntax.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Thank you for testing this, and please don't worry about the timing. You
found a data loss bug, which is worth much more to me than a fast reply.

You were right, and it was worse than the symptom you described. I rebuilt
v3 to check. With an inheritance parent and child both subscribed:

publisher: ip (own rows) = 2, ic = 2

v3, ALTER SUBSCRIPTION s REFRESH TABLE ip:
after refresh: ip ONLY = 0, ic = 0 <- child truncated too
catalog state: ip = 'i', ic = 'r' <- only ip reset
after enable: ip ONLY = 2, ic = 0 <- child never copied back

The child's rows were gone permanently. v3 passed the parse tree's
RangeVars to ExecuteTruncate(), and qualified_name produces RangeVars with
inh = true, so the truncation recursed into inheritance children while the
state reset only touched the relations named in the command. That is
exactly the mismatch in your fourth point.

The rule I should have written down at the start, and which is now a
comment in the code, is: never discard a relation unless we also reset the
state of whatever will refill it.

Rewriting this function to call ExecuteTruncateGuts() with resolved OIDs,
which Sawada-san had asked for on this thread, fixes it as a side effect,
because recursion is then driven by relkind and only follows partitions.
The same scenario on v4 ends at ip ONLY = 2, ic = 2, matching the
publisher.

its current behavior is equivalent to the TRUNCATE ONLY syntax

For inheritance that is what v4 does deliberately, though v3 did the
opposite. Following children is not safe: a child is an independent
relation that may be subscribed in its own right, or may hold data that is
not replicated at all, so truncating it from the parent's refresh throws
away rows nothing will copy back. Naming both refreshes both:

ALTER SUBSCRIPTION s REFRESH TABLE parent, child;

Partitions are different and are still followed, because a partitioned
table is only tracked when the publication uses publish_via_partition_root,
and then its partitions are not tracked separately, so the root's re-copy
refills them by tuple routing.

The partitioned parent table cannot be refreshed directly

Working as intended, but the error gave you no way to work that out, which
is fair. Without publish_via_partition_root the subscription tracks the
partitions, so naming the root hits "is not part of the subscription". v4
adds a hint:

HINT: The publication may publish its partitions individually; refresh
those instead.

SELECT * FROM parent returns only residual child rows during resync

Real and expected on v4: the child's rows are still valid replicated data
and are left alone, while the parent's own rows are re-copied. The broader
point, that a refreshed table is visibly empty until its sync finishes, is
inherent to truncate-and-recopy. Removing it needs the copy to go into a
new relfilenode which is then swapped in, like REFRESH MATERIALIZED VIEW
CONCURRENTLY. I would rather keep that out of this patch, but it is the
natural follow-up.

A separate review then found the same class of bug across subscriptions: a
relation that another subscription also populates was truncated without
that subscription's state being reset. v4 rejects that up front.

v4 is posted in my reply to Masahiko on this thread, so I am not
duplicating the attachment. v3 was the last version posted and three
reviews landed on it, so v4 folds in all of them at once. The parts that
came from your report are the inheritance fix above, the hint for an
untracked partitioned root, documentation of the partition and inheritance
behaviour, and new regression checks.

Thanks again for catching this.

Regards,
Cagri Biroglu

On Tue, Aug 11, 2026 at 12:21 PM Yilin Zhang <jiezhilove@126.com> wrote:

Show quoted text

At 2026-08-10 16:53:04, "Cagri Biroglu" <cagri.biroglu@adyen.com> wrote:

Hi again,
Thanks, and thanks for picking this up. I've registered it in the open
CommitFest (PG20-2).
Attached is v3, which is v2 rebased onto current master. Master removed
Subscription.conninfo, so the one code change is that the mid-sync path
now resolves the connection string with SubscriptionConninfo(sub). It
does that at the point of use rather than up front, because the common
case here is a relation in ready state, which needs no publisher
connection at all. Nothing else changed, so your review of v2 still
applies.

hi,
I have some review feedback on the v3-0001 patch.
The handling of inheritance tables and partition child tables with REFRESH
TABLE seems incomplete; its current behavior is equivalent to the TRUNCATE
ONLY syntax.
This could result in the following problems:
After executing REFRESH TABLE and before the tablesync worker completes
synchronization, running SELECT * FROM parent will only return residual
data from child tables, while rows residing directly in the parent table
will be missing.
The partitioned parent table cannot be refreshed directly.
After the subscription is disabled, the states of child tables are not
reset.
I originally expected data conflicts would arise, yet testing showed no
critical data anomalies, and the data eventually became consistent.
Could you please assess whether this constitutes an issue that needs
addressing?
Besides, I began reviewing this patch yesterday but have progressed rather
slowly. The patch is now tagged "Waiting on Author". I apologize if this
has held up your development work.

Best regards,
--
Yilin Zhang

Dear Hayato,

Thank you for testing, and especially for the reproducer.

One note on versions first, to save you looking for something that is not
there: v3 is the last version I posted, and three reviews landed on it,
including yours. v4 folds in all of them at once, so it is a larger step
than usual. The v3 to v4 delta is:

- drop the tablesync slots last, over a single publisher connection
- reject the command when two_phase is enabled (your 01)
- take AccessExclusiveLock when the relations are resolved
- use ExecuteTruncateGuts() with resolved OIDs, plus an explicit
TRUNCATE privilege check, which that function does not do
- wait for all worker types, not just the apply worker (your 02)
- reject relations another subscription also populates (your 03)
- stop following inheritance children, which lost their rows
- hint when a partitioned root is not tracked by the subscription
- psql tab completion, and documentation, which v3 had none of

Taking your points in turn.

01. two_phase

Already in v4, added for Masahiko's review with the same reasoning as
yours. There is no "&& opts.copy_data" term, since REFRESH TABLE always
copies and so has no exception to carve out. I used
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE rather than ERRCODE_SYNTAX_ERROR,
to match the adjacent state checks such as "cannot disable two_phase when
prepared transactions exist"; nothing is syntactically wrong here. Happy
to switch if you prefer it to match REFRESH PUBLICATION exactly.

On the prepared transaction stall: with this guard it is unreachable,
because subscription gxacts only exist once two_phase is ENABLED, and
two_phase cannot be turned off while they exist. Note this means your
reproducer stops early on v4, since both subscriptions use two_phase = on;
I removed that to reach point 03.

02. check the leader worker too, and drop the stopping code

Agreed and adopted, and this closed a hole I had not seen. The check I had
written for first review covered only WORKERTYPE_APPLY, and a tablesync
worker outlives its apply worker: on a 1 GB table the apply worker is gone
within a second of DISABLE while the tablesync worker keeps copying for
another 12. So that check could pass while a tablesync worker still held
the relation.

It now uses logicalrep_workers_find(subid, false, true) and the
logicalrep_worker_stop() loop is gone. I chose only_running = false, as
DROP SUBSCRIPTION does, because a worker that is in_use but has not
attached yet is the dangerous one: it is about to read the relation state
we are rewriting.

03. TRUNCATE happens but srsubstate cannot be updated

Confirmed. Your case with two_phase removed:

before: foo_1 has 9 rows, a=2,4,6,8 from sub2 and a=15,16,18,19,20
routed by sub1
after REFRESH TABLE foo on sub1, then ENABLE: foo_1 has 5 rows

sub2's four rows were destroyed and never copied back, because sub2's
state stayed 'r'.

Fixing it, I found the problem is more general than partitioning. The same
loss happens with no partitions at all, when two subscriptions carry
disjoint row filters on one table:

CREATE PUBLICATION p_lo FOR TABLE t WHERE (a < 10);
CREATE PUBLICATION p_hi FOR TABLE t WHERE (a >= 10);
-- s_lo and s_hi on the subscriber, 20 rows
ALTER SUBSCRIPTION s_hi DISABLE;
ALTER SUBSCRIPTION s_hi REFRESH TABLE t; -- truncates all 20
ALTER SUBSCRIPTION s_hi ENABLE;
-- t has 11 rows; s_lo's 9 are gone

So the rule the patch was missing is that a relation must not be discarded
unless the state of whatever will refill it is reset too. A relation is
refilled by any subscription that tracks it directly, and, if it is a
partition, by any subscription that tracks a partitioned ancestor and
routes rows into it. v4 checks that before touching anything, so the
command stays all-or-nothing, and rejects instead of losing data:

ERROR: table "t" is also part of the subscription "s_lo"
ERROR: table "foo_1" is also part of the subscription "sub2"
ERROR: table "pp_1" is a partition of "pp", which is part of the
subscription "s_root"

The last is the reverse of your case: naming the partition while another
subscription tracks the root.

I chose to refuse rather than reset the other subscription's relations
too. Doing that properly would mean requiring the other subscription to be
disabled as well, resetting catalog rows belonging to it, and dropping its
tablesync slots over its own connection, all from a command naming a
different subscription. That felt like a bigger decision than I should
make alone, so v4 turns silent data loss into a clear error. If you think
the cross-subscription reset is the right end state, I am glad to write it.

Six regression checks cover the three shapes, including that the data is
still intact after the command is rejected. Against the previous code five
of them fail, among them both "no rows lost" assertions.

Best regards,
Cagri Biroglu

On Fri, Aug 14, 2026 at 12:18 PM Hayato Kuroda (Fujitsu) <
kuroda.hayato@fujitsu.com> wrote:

Show quoted text

Dear Cagri,

Thanks for updating the patch. I read and tested your patch. Here are my
comments.

01.
According to the REFRESH PUBLICATION command, we must prohibit to
synchronize
tables if the two_phase is enabled, but REFRESH TABLE seems to bypass the
restriction.
Also, if there are prepared transactions done by the subscription and they
modify
target relations, the command would stuck forever - the apply worker won't
start
again. So should we have the same guard as the REFRESH PUBLICATION?

02.
You have already checked the case the existence of tablesync workers, but
not for the leader worker. I feel we should ensure via
logicalrep_workers_find()
like ALTER SUBSCRIPTION SET (two_phase). Also, we can remove the part from
AlterSubscription_refresh_table().

03.
Regarding the partition table, I found the case that TRUNCATE happened but
srsubstate cannot be updated. It's because ExecuteTruncate() truncates
tables all
child tables but AlterSubscription_refresh_table() updates tuples relid is
exactly matched. Per my experiment, this can cause the issue if the
different publications
publish the root and child separately, and they are subscribed by the
different subscription.
See attached reproducer.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Dear Cagri,

Thanks for the update. I found this patch could cause an issue if two subscriptions
are modifying the same table. The scenario:

1. A tablesync worker for sub1 acquired opened a table with the RowExclusive in LogicalRepSyncTableStart().
2. User ran ALTER SUBSCRIPTION sub2 REFRESH TABLE command.
3. The backend acquired a AccessExclusive lock for pg_subscription_rel, then
tried to acquire a AccessExclusive Lock for tables. It would wait till the tablesync
worker released.
4. The tablesync worker also tried to acquire AccessShare lock for pg_subscription_rel,
but it would be blocked by the backend. It's done in
copy_table()->logicalrep_rel_open()->GetSubscriptionRelState().
5. The deadlock detector detected the wait-for graph is now circle,
thus it terminates either of them.

One idea for the fix is to acquire AccessShare locks for user-defined tables
first, then acquire the AccessExclusive lock after the CheckRefreshTableNotShared().
This avoids to acquire strong locks only if it's needed, and my reproducer can
reject by the function. Thought?

Also, below are my cosmetic comments.

01.
```
+ LOCKMODE lockmode = AccessExclusiveLock;
```

It might be matter of taste, but I feel the variable lock_mode is not needed:
the pattern is mainly used when the mode can be different based on situations.

02.
```
+static void
+CheckRefreshTableNotShared(Relation pgsubrel, Subscription *sub,
+                                                  List *subrelids, List *relids)
```

The name CheckRefreshTableNotShared() is not suitable, because not sure the
meaning "Shared". How about CheckRefreshTableNotInOtherSubscriptions?

03.
```
+               /*
+                * Tuples reach a partition through its ancestors, so a subscription
+                * tracking any of them keeps this relation populated too.  A named
+                * relation's own entry belongs to this subscription and is skipped
+                * below by the srsubid test.
+                */
+               feeders = lappend_oid(get_partition_ancestors(relid), relid);
```

get_partition_ancestors() seems to assume that the given relation has at least
one parent, but the patch does not ensure. Maybe get_rel_relispartition() or
similar functions can be used.

04.
You missed to update meson.build file.

05.
```
+$node_subscriber->append_conf('postgresql.conf',
+       "wal_retrieve_retry_interval = 1ms");
```

Can you clarify the reason why the setting is required? Can we remove if not needed?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hello Hayato,

Thank you for the review, v5 is attached.

I found that the deadlock could happen between the tablesync worker and

the

REFRESH TABLE command.

Confirmed. Reproduced with a 3M row table so the copy is slow enough to
overlap: sub2 syncs and is disabled, sub1 starts copying the same table,
then
ALTER SUBSCRIPTION sub2 REFRESH TABLE t runs. On v4:

process 35095 still waiting for AccessExclusiveLock on relation 16384
of database 5 after 1000.458 ms
process 35072 detected deadlock while waiting for RowExclusiveLock on
relation 6102 of database 5 after 1001.136 ms
process 35095 acquired AccessExclusiveLock on relation 16384 of
database 5 after 4311.560 ms
process 35072 ERROR: deadlock detected

16384 is the table, 6102 is pg_subscription_rel. Worth spelling out who
died:
35072 was sub1's tablesync worker. The command killed a worker of a
subscription it was not operating on, and one it was going to refuse to
touch
anyway.

One reason this is easy to miss: the command's own output is identical on
both
builds. It reports the shared-relation rejection either way, and on v4 it
then
goes on to acquire the lock and finish. Only the server log shows that a
worker was terminated, which is why the script greps for it rather than
trusting the psql output.

4. The tablesync worker also tried to acquire AccessShare lock for
pg_subscription_rel, but it would be blocked by the backend. It's done in
copy_table()->logicalrep_rel_open()->GetSubscriptionRelState().

One small refinement, which only broadens your point. In my run the worker
was blocked on RowExclusiveLock rather than AccessShareLock, so it had got
past the read in GetSubscriptionRelState() and was updating its own state
via
UpdateSubscriptionRelState(). Both levels conflict with AccessExclusiveLock,
so the cycle is the same one; it just means any catalog access by the worker
closes it, read or write, and not only the call path you traced.

One idea for the fix is to acquire AccessShare locks for user-defined

tables

first, then acquire the AccessExclusive lock after the
CheckRefreshTableNotShared(). This avoids to acquire strong locks only if
it's needed, and my reproducer can reject by the function. Thought?

Agreed, and adopted. Relations are resolved with AccessShareLock, the check
runs, and only then:

foreach_oid(relid, relids)
LockRelationOid(relid, AccessExclusiveLock);

Your reproducer then rejects immediately with the shared-relation error,
sub1
keeps copying, and the log has no deadlock lines. It is also the better
shape
for the reason you give: the common rejection no longer waits for somebody
else's copy to finish before failing.

Three things I ran into while testing it, of which the second is the reason
v5 goes further than your suggestion.

First, the escalation makes this a lock upgrade, so it is worth showing that
two commands cannot sit on AccessShareLock and both ask for
AccessExclusiveLock. They cannot, for a structural reason rather than by
luck. AlterSubscription() takes

/* Lock the subscription so nobody else can do anything with it. */
LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock);

before the switch on stmt->kind, so two REFRESH TABLE commands on the same
subscription are serialized before either touches a relation. Two commands
on
different subscriptions can only collide over a relation both subscriptions
feed, which is exactly what the check rejects before the escalation.
Measured
with two concurrent commands and the relation pinned by a third session:

locks on t:
72213 | AccessShareLock | t
72213 | AccessExclusiveLock | f
lock on the subscription object (classid 6100):
72213 | AccessExclusiveLock | t
72214 | AccessExclusiveLock | f

The second command never acquires AccessShareLock on t at all; it waits on
the
object lock.

Second, and this is the part that matters: your fix does not remove the
deadlock class on its own. The command still held AccessExclusiveLock on
pg_subscription_rel while waiting for the relation, and that alone is
enough.
Any backend holding a lock on one of these relations that then reads
pg_subscription_rel closes the same cycle, and it need not be a replication
worker:

BEGIN;
INSERT INTO t VALUES (-1); -- RowExclusiveLock on t
<pause>
SELECT count(*) FROM pg_subscription_rel;

run against REFRESH TABLE on a relation that passes the check, so the
escalation is actually reached:

Process 38605 waits for AccessShareLock on relation 6102 of database 5;
blocked by process 38722.
Process 38722 waits for AccessExclusiveLock on relation 16384 of
database 5; blocked by process 38605.
ERROR: deadlock detected

This time the user's transaction was the victim. No ordering of the two
locks
fixes this: whichever we take first, a transaction taking them in the other
order deadlocks. So v5 also weakens the catalog lock to the level this
function's updates actually need:

rel = table_open(SubscriptionRelRelationId, RowExclusiveLock);

I had used AccessExclusiveLock to match AlterSubscription_refresh(), but the
two are not comparable: that function never waits for a lock on a user
relation while holding the catalog, and this one does. RowExclusiveLock does
not conflict with the AccessShareLock or RowExclusiveLock that workers and
ordinary backends take there, so the cycle cannot form. The same test now
commits normally and the command is accepted, with only an ordinary lock
wait
logged.

Third, that weakening is not free, and it is the one judgement call here I
would like your opinion on. RowExclusiveLock is also what CREATE
SUBSCRIPTION
takes, so it is no longer serialized against us, and CREATE SUBSCRIPTION ...
WITH (copy_data = false) records relations as SUBREL_STATE_READY. A relation
registered that way is never copied, so a registration landing after the
check
would leave the truncate discarding rows that nothing brings back.

What prevents it is that CREATE SUBSCRIPTION locks the relation before
registering it:

relid = RangeVarGetRelid(rv, AccessShareLock, false);
...
AddSubscriptionRelState(subid, relid, relation_state, ...);

so once this command has requested AccessExclusiveLock, no registration for
that relation can complete. Verified rather than assumed:

query
| mode | granted

-----------------------------------------------------------------+---------------------+--------
ALTER SUBSCRIPTION s1 REFRESH TABLE t | AccessShareLock | t
SELECT pg_sleep(8); |
AccessShareLock | t
ALTER SUBSCRIPTION s1 REFRESH TABLE t | AccessExclusiveLock | f
CREATE SUBSCRIPTION s2 CONNECTION ... | AccessShareLock | f

That leaves only the instant between the check's scan and the lock request,
so
v5 runs the check a second time once the relations are locked. The first
pass
is then purely the optimisation you asked for, and the second is the
conclusive one. REFRESH PUBLICATION still takes AccessExclusiveLock and so
remains serialized against us either way.

If you would rather keep AccessExclusiveLock on the catalog and accept the
user-transaction deadlock, it is a one line change and I will make it. I
preferred fixing the deadlock, since it can hit a transaction that has
nothing
to do with logical replication.

01. It might be matter of taste, but I feel the variable lock_mode is not
needed: the pattern is mainly used when the mode can be different based on
situations.

Removed. There are genuinely two levels now, and naming them at the sites
that
take them reads better than one variable whose meaning changes halfway
through.

02. The name CheckRefreshTableNotShared() is not suitable, because not

sure

the meaning "Shared". How about CheckRefreshTableNotInOtherSubscriptions?

Renamed as you suggest.

03. get_partition_ancestors() seems to assume that the given relation has

at

least one parent, but the patch does not ensure. Maybe
get_rel_relispartition() or similar functions can be used.

You were right that this call needs a relispartition test, and
get_rel_relispartition() is what v5 now uses. The reason turned out to be a
different one than either of us had in mind, and it was a live bug, so let
me
lay out both halves.

It does not error on a relation with no parent. get_partition_ancestors()
calls get_partition_ancestors_worker(), which treats that as its base case,
because the top of any partition tree has no parent either:

/*
* Recursion ends at the topmost level, ie., when there's no parent; also
* when the partition is being detached.
*/
parentOid = get_partition_parent_worker(inhRel, relid, &detach_pending);
if (parentOid == InvalidOid || detach_pending)
return;

and get_partition_parent_worker() initialises its result to InvalidOid,
overwriting it only if the scan finds a tuple. The elog(ERROR) calls are in
get_partition_parent(), a different wrapper over the same worker, which this
path never calls.

The actual problem is what it returns when there is a parent but not a
partitioning one. get_partition_parent_worker() scans pg_inherits on
(inhrelid, inhseqno = 1) and does not look at relispartition, and
pg_inherits
records plain inheritance too. So for an INHERITS child it returns the
inheritance parent, and my code was treating that as a feeder. An
inheritance
parent does not route rows into its children, so that is wrong, and it is
observable: with tab_inh_c inheriting from tab_inh_p, and a second
subscription tracking ONLY tab_inh_p,

ALTER SUBSCRIPTION tap_sub_inh REFRESH TABLE tab_inh_c;
ERROR: table "tab_inh_c" is a partition of "tab_inh_p", which is part of
the subscription "tap_sub_inh_ponly"

A valid refresh is refused, and the message calls an inheritance child a
partition. It errs on the safe side, so no data was at risk, but there is no
way for the user to work around it.

v5 restricts the ancestor walk to real partitions:

if (get_rel_relispartition(relid))
feeders = lappend_oid(get_partition_ancestors(relid), relid);
else
feeders = list_make1_oid(relid);

The case above is now accepted, the genuine partition-ancestor rejection
still
fires, and 039 has a test for it that fails without the guard. Thank you for
pushing on this one; I had convinced myself it was only a style question.

04. You missed to update meson.build file.

Done.

05. Can you clarify the reason why the setting is required? Can we remove

if

not needed?

Removed. I had copied it from 004_sync.pl.

Best regards,
Cagri

On Mon, Aug 17, 2026 at 11:09 AM Hayato Kuroda (Fujitsu) <
kuroda.hayato@fujitsu.com> wrote:

Show quoted text

Dear Cagri,

Thanks for the update. I found this patch could cause an issue if two
subscriptions
are modifying the same table. The scenario:

1. A tablesync worker for sub1 acquired opened a table with the
RowExclusive in LogicalRepSyncTableStart().
2. User ran ALTER SUBSCRIPTION sub2 REFRESH TABLE command.
3. The backend acquired a AccessExclusive lock for pg_subscription_rel,
then
tried to acquire a AccessExclusive Lock for tables. It would wait till
the tablesync
worker released.
4. The tablesync worker also tried to acquire AccessShare lock for
pg_subscription_rel,
but it would be blocked by the backend. It's done in
copy_table()->logicalrep_rel_open()->GetSubscriptionRelState().
5. The deadlock detector detected the wait-for graph is now circle,
thus it terminates either of them.

One idea for the fix is to acquire AccessShare locks for user-defined
tables
first, then acquire the AccessExclusive lock after the
CheckRefreshTableNotShared().
This avoids to acquire strong locks only if it's needed, and my reproducer
can
reject by the function. Thought?

Also, below are my cosmetic comments.

01.
```
+ LOCKMODE lockmode = AccessExclusiveLock;
```

It might be matter of taste, but I feel the variable lock_mode is not
needed:
the pattern is mainly used when the mode can be different based on
situations.

02.
```
+static void
+CheckRefreshTableNotShared(Relation pgsubrel, Subscription *sub,
+                                                  List *subrelids, List
*relids)
```

The name CheckRefreshTableNotShared() is not suitable, because not sure the
meaning "Shared". How about CheckRefreshTableNotInOtherSubscriptions?

03.
```
+               /*
+                * Tuples reach a partition through its ancestors, so a
subscription
+                * tracking any of them keeps this relation populated
too.  A named
+                * relation's own entry belongs to this subscription and
is skipped
+                * below by the srsubid test.
+                */
+               feeders = lappend_oid(get_partition_ancestors(relid),
relid);
```

get_partition_ancestors() seems to assume that the given relation has at
least
one parent, but the patch does not ensure. Maybe get_rel_relispartition()
or
similar functions can be used.

04.
You missed to update meson.build file.

05.
```
+$node_subscriber->append_conf('postgresql.conf',
+       "wal_retrieve_retry_interval = 1ms");
```

Can you clarify the reason why the setting is required? Can we remove if
not needed?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Dear Cagri,

Hi, thanks for updating the patch. Sorry I could not reply to your latest post,
because it was blocked by the company's rule.

Few comments:
01.
According to ExecuteTruncate(), we did some additional check like
truncate_check_rel() and truncate_check_perms(). Is it needed here? If not we can
add code comment here. At least, InvokeObjectTruncateHoo() seems needed.

02. Question:
This command drops replication origins and remote slots, which cannot be rolled back.
So what should users do the command fails?

03.
Also, such operations should be done after the transctional ones (ExecuteTruncateGuts).

04.
I found that ATTACH PARTITION/DETACH PARTITION CONCURRENTLY command acquires the
ShareUpdateExclusive Lock for the root table. This can cause that tables are
missed to be truncated, or detached tables can be truncated.
One primitive idea for fixing is to check whether the schema structure is modified
after acquiring the AccesExclusive lock, but there may be better approaches.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Wed, Aug 19, 2026 at 9:10 PM Cagri Biroglu <cagri.biroglu@adyen.com> wrote:

Hello Hayato,

Thank you for the review, v5 is attached.

Hello Cagri,

Some minor comments:
1. The header above CheckRefreshTableNotInOtherSubscriptions() has two
sets of comments, the first one I feel is meant for
AlterSubscription_refresh_table() which is without a header.

2.
+ /*
+ * The local copy is discarded below, so require the same privilege
+ * TRUNCATE itself would.
+ */
+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(get_rel_relkind(relid)),
+   rv->relname);

rather than doing it this way, I believe you need to call
truncate_check_rel(relid, rel->rd_rel);
truncate_check_perms(relid, rel->rd_rel);
truncate_check_activity(rel);

else you will miss out on checking for temp tables of other backends
and invoking InvokeObjectTruncateHook(relid) which are object access
hooks.

regards,
Ajin Cherian
Fujitsu Australia

Dear Hayato,

Thank you .

v6 is attached, along with the reproducer for point 04.

01. Can you clarify the reason why ExecuteTruncate() is not used here?
It does truncate_check_rel() and truncate_check_perms(). Are they
needed here? If not, please add a comment. At least,
InvokeObjectTruncateHoo() seems needed.

ExecuteTruncate() is not usable because it starts from a list of RangeVars
and re-resolves them, whereas by that point we have already resolved,
validated and locked the relations, and the whole set has to be truncated in
one call so that foreign keys between its members are satisfied.

On the checks themselves, you are right that some of them were missing.
ExecuteTruncateGuts() applies them only to the relations it discovers itself
through CASCADE and leaves the explicitly passed ones to the caller, so as a
direct caller we owe them.

Going through what they do:

truncate_check_rel() rejects non-tables and system catalogs. Neither can
occur for a relation that is in pg_subscription_rel, and sequences are
rejected earlier with a more helpful message.

truncate_check_perms() is the ACL_TRUNCATE check. The first pass already
does this explicitly.

truncate_check_activity() rejects other backends' temp tables and calls
CheckTableNotInUse(). The temp case cannot arise, as a temp table cannot
be
a partition of a permanent one, but CheckTableNotInUse() was missing.

InvokeObjectTruncateHook(), which as you say was missing.

Worth noting for the archives: the hook is not a separate call. It is the
last statement of truncate_check_rel(), so a caller only fires it by going
through that function. apply_handle_truncate() calls none of the three, and
therefore does not fire the hook either. I still think you are right that we
should. That path is replication applying a remote TRUNCATE; ours is a user
typing a command that discards the contents of a table, which is precisely
what an audit or label provider wants to see, and it is also why we already
require TRUNCATE privilege where apply does not.

02. Question: this command drops replication origins and remote slots,
which cannot be rolled back. So what should users do if the command
fails?

03. Such operations should be done after the transactional ones
(i.e., ExecuteTruncateGuts()).

Taking 03 first, since it changes the answer to 02.

You are right, and the origin drop was worse placed than it looks.
replorigin_drop_by_name() is half transactional: CatalogTupleDelete() rolls
back, but replorigin_state_clear() emits XLOG_REPLORIGIN_DROP and clears the
in-memory progress immediately, and that part does not roll back. So the old
ordering could wipe an origin's progress and then abort in the truncate, on
nothing more exotic than a foreign key reference or a lock timeout, leaving
the origin row restored but its progress gone.

v6 moves the origin drops after ExecuteTruncateGuts() and after the state
resets, so the order is now: everything that can fail for an ordinary
reason,
then the origin drops, then the slot drops last of all as the only step
whose
effect leaves this node.

That gives 02 a fairly narrow answer. Every ordinary failure , name
resolution, subscription membership, privileges, the cross-subscription
check, the truncate, the state resets ; now happens before anything
irreversible, so the command is all-or-nothing and there is nothing for the
user to do but fix the cause and re-run.

What remains is a failure inside the final phase itself, for instance the
publisher becoming unreachable partway through the slot drops. Then the
transaction rolls back, the catalog is unchanged, and some tablesync slots
are already gone. The answer there is simply to re-run the command: the
slots
are dropped with missing_ok, so the ones already gone are skipped and the
command converges. This is the same bargain DROP SUBSCRIPTION makes, and for
the same reason, which is also why both are PreventInTransactionBlock().

04. IIUC ATTACH PARTITION and DETACH PARTITION CONCURRENTLY require
only ShareUpdateExclusiveLock for the root table. This means that
some tables can be missed from the truncation, or detached tables
can be truncated. A primitive idea is to check whether the schema
structure is modified after acquiring the AccessExclusive lock;
there may be better approaches.

Confirmed, and fixed as you suggest.

Best regards,
Cagri Biroglu

On Fri, Aug 21, 2026 at 5:54 AM Hayato Kuroda (Fujitsu) <
kuroda.hayato@fujitsu.com> wrote:

Show quoted text

Dear Cagri,

Hi, thanks for updating the patch. Sorry I could not reply to your latest
post,
because it was blocked by the company's rule.

Few comments:
01.
According to ExecuteTruncate(), we did some additional check like
truncate_check_rel() and truncate_check_perms(). Is it needed here? If not
we can
add code comment here. At least, InvokeObjectTruncateHoo() seems needed.

02. Question:
This command drops replication origins and remote slots, which cannot be
rolled back.
So what should users do the command fails?

03.
Also, such operations should be done after the transctional ones
(ExecuteTruncateGuts).

04.
I found that ATTACH PARTITION/DETACH PARTITION CONCURRENTLY command
acquires the
ShareUpdateExclusive Lock for the root table. This can cause that tables
are
missed to be truncated, or detached tables can be truncated.
One primitive idea for fixing is to check whether the schema structure is
modified
after acquiring the AccesExclusive lock, but there may be better
approaches.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hello Ajin,

Thank you for the review. This needs a change outside the feature;
v7 is attached.

1. The header above CheckRefreshTableNotInOtherSubscriptions() has two
sets of comments, the first one I feel is meant for
AlterSubscription_refresh_table() which is without a header.

Right , done.

2.
+ /*
+ * The local copy is discarded below, so require the same privilege
+ * TRUNCATE itself would.
+ */
+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(get_rel_relkind(relid)),
+   rv->relname);

rather than doing it this way, I believe you need to call
truncate_check_rel(relid, rel->rd_rel);
truncate_check_perms(relid, rel->rd_rel);
truncate_check_activity(rel);

else you will miss out on checking for temp tables of other backends
and invoking InvokeObjectTruncateHook(relid) which are object access
hooks.

Agreed. v6 does call the hook, but by hand;
v7 does it the way you describe, which also brings in the temp table check
that the hand-rolled version was still missing.

So v7 makes the three functions extern, which is no functional change on its
own, and calls them as you describe:

truncate_check_rel(relid, userrel->rd_rel);
truncate_check_activity(userrel);

for every relation that will be truncated, and

truncate_check_perms(relid, userrel->rd_rel);

for the relations actually named in the command. The privilege check stays
in
the first pass, earlier than the other two, so that the command is rejected
before it escalates any lock. It is deliberately not applied to the
partitions
that come along with a named partitioned table, which follows what
ExecuteTruncate() does for inheritance children:

* Inherited TRUNCATE commands perform access permission
* checks on the parent table only. So we skip checking the
* children's permissions and don't call
* truncate_check_perms() here.
*/
truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);
truncate_check_activity(rel);

Two consequences are behaviour changes rather than tidying, so they are
worth
naming.

Your point about other backends' temp tables was the substantive one. The
check was being applied only to the partitions the command collects, on the
argument that a temp table cannot be a partition of a permanent one; the
relations named in the command were not covered at all, which left that case
resting on an untested assumption about what can appear in
pg_subscription_rel. That is not the kind of thing to lean on.
truncate_check_activity() now covers them, and errors rather than skips, as
ExecuteTruncate() does for explicitly named relations.

truncate_check_activity() calls CheckTableNotInUse(rel, "TRUNCATE"), so that
message now says TRUNCATE rather than naming the subscription command. I
preferred that to keeping a hand-rolled copy for the sake of one string, but
say the word if you would rather it read ALTER SUBSCRIPTION ... REFRESH
TABLE
and I will thread the statement name through.

Best regards,
Cagri Biroglu

On Tue, Aug 25, 2026 at 7:52 AM Ajin Cherian <itsajin@gmail.com> wrote:

Show quoted text

On Wed, Aug 19, 2026 at 9:10 PM Cagri Biroglu <cagri.biroglu@adyen.com>
wrote:

Hello Hayato,

Thank you for the review, v5 is attached.

Hello Cagri,

Some minor comments:
1. The header above CheckRefreshTableNotInOtherSubscriptions() has two
sets of comments, the first one I feel is meant for
AlterSubscription_refresh_table() which is without a header.

2.
+ /*
+ * The local copy is discarded below, so require the same privilege
+ * TRUNCATE itself would.
+ */
+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(get_rel_relkind(relid)),
+   rv->relname);

rather than doing it this way, I believe you need to call
truncate_check_rel(relid, rel->rd_rel);
truncate_check_perms(relid, rel->rd_rel);
truncate_check_activity(rel);

else you will miss out on checking for temp tables of other backends
and invoking InvokeObjectTruncateHook(relid) which are object access
hooks.

regards,
Ajin Cherian
Fujitsu Australia