Subject: Re-read subscription state after lock in AlterSubscription

From: Bertrand Drouvot <[email-masked]>
Date: Thu, 2 Jul 2026 12:08:06 +0000

Hi hackers,

while playing with the new ALTER SUBSCRIPTION parameter added in a5918fddf10,
I realized that the subscription is not re-read once we acquire the lock in
AlterSubscription().

This pre-existing issue is now more visible after a5918fddf10:

1/ two concurrent ALTER SUBSCRIPTION SET (conflict_log_destination = 'table')
could result in the second session attempting to create an already-existing
conflict log table, producing a confusing "relation already exists" error:

ERROR:  relation "pg_conflict_log_24614" already exists

It's confusing because ALTER SUBSCRIPTION SET (conflict_log_destination = 'table')
would not report an error if the conflict table already exists (and no concurrent
ALTER is running).

2/ a concurrent DROP followed by the ALTER would emit a NOTICE about creating the
conflict log table before failing with "referenced subscription was concurrently
dropped". That sounds like a weird messaging:

NOTICE:  created conflict log table "pg_conflict.pg_conflict_log_24620" for subscription "mysub"
ERROR:  referenced subscription was concurrently dropped

The attached fixes it by:

- Re-reading the subscription tuple after LockSharedObject() and refreshing the
  Subscription struct.
- Moving the local variable assignments to after the re-read.
- Re-checking the password_required privilege restriction after the re-read.

Remarks:

1/ not re-checking password_required after the re-read would still produce a
"tuple concurrently updated" error, but re-checking it allows us to display a
better error message.

2/ the ownership check is intentionally not re-done after the lock because
AlterSubscriptionOwner() does not take AccessExclusiveLock on the subscription
object: it only takes RowExclusiveLock on the pg_subscription catalog table.
This means ownership can change regardless of our lock, making a re-check after
lock acquisition pointless. The existing "tuple concurrently updated" error from
CatalogTupleUpdate() already provides a protection if ownership changes
concurrently.

3/ the "privileges" checks are still also done before the lock acquisition because
we don't want to lock an object we don't have privileges on.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

---

From: Dilip Kumar <[email-masked]>
Date: Thu, 2 Jul 2026 17:57:09 +0530

> we don't want to lock an object we don't have privileges on.

Thanks Bertrand, yeah this seems like a valid issue, and I agree we
need to reread the subscription after acquiring the object lock.

-- 
Regards,
Dilip Kumar
Google

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 2 Jul 2026 12:48:53 +0000

Dear Bertrand,

Good catch. Current code allows that old `sub` value is retained, so it sounds
reasonable fix even for me.

BTW, the issue that GetSubscription() is called before the LockSharedObject() looks
the existing issues even on REL_13_STABLE. So does it mean that there were no
cases that concurrent altering can be the unexpected state? At least,
"retain_dead_tuples" can avoid the issue because the launcher manages the
conflict slot.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Bertrand Drouvot <[email-masked]>
Date: Thu, 2 Jul 2026 13:20:45 +0000

Hi Kuroda-san,

On Thu, Jul 02, 2026 at 12:48:53PM +0000, Hayato Kuroda (Fujitsu) wrote:
> cases that concurrent altering can be the unexpected state?

Yeah, but I think they would produce "tuple concurrently updated" error (due to 
CatalogTupleUpdate) so that invalid information could not be used.

Regards,

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Fri, 3 Jul 2026 03:13:08 +0000

Dear Bertrand,

> CatalogTupleUpdate) so that invalid information could not be used.

I confirmed with PG14 that tuple concurrently updated ERROR can be raised when
ALTER SUBSCRIPTION DISABLE happens concurrently:

```
postgres=# ALTER SUBSCRIPTION sub DISABLE ;
ERROR:  tuple concurrently updated
```

It might be harmless but I think the correct ERROR should be reported: the patch
should be backpatched. Thought?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 3 Jul 2026 04:19:00 +0000

Hi Kuroda-san,

> ERROR:  tuple concurrently updated

Yeah, reproducible by using a breakpoint just before acquiring the lock for example.

> should be backpatched. Thought?

I'm not sure about the back patch part as it would only improve error messages
in a rare race condition (and there is no risk of invalid data being used).

Since a5918fddf10, that's a different story because a table creation is now
involved.

Regards,

---

From: Dilip Kumar <[email-masked]>
Date: Fri, 3 Jul 2026 10:20:32 +0530

> in a rare race condition (and there is no risk of invalid data being used).

Patch LGTM. IMHO we can backpatch this as it is a small change and
also fixes the bug, without this fix a non-superuser executing  ALTER
SUBSCRIPTION could bypass the password_required=false restriction if a
concurrent transaction
updated that flag.  However, we could argue that this is a corner case
and can be skipped but given the patch's simplicity, I recommend
backpatching.

-- 
Regards,
Dilip Kumar
Google