On Fri, Jul 25, 2025 at 6:01 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Ajin,
>
> Thanks for patches! While checking, I recalled that the backpatch policy [1].
> We must not modify definitions of opened functions but this does. Can you define
> another function like UpdateSubscriptionRelStateEx or something on the back
> branches?
>
> Another comment:
> ```
> @@ -328,9 +328,13 @@ UpdateSubscriptionRelState(Oid subid, Oid relid, char state,
> Datum values[Natts_pg_subscription_rel];
> bool replaces[Natts_pg_subscription_rel];
>
> - LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock);
> -
> - rel = table_open(SubscriptionRelRelationId, RowExclusiveLock);
> + if (already_locked)
> + rel = table_open(SubscriptionRelRelationId, NoLock);
> ```
>
> Can we assert that RowExclusiveLock for pg_subscription_rel has already been
> acquired, by using CheckRelationOidLockedByMe() family?
>
Thanks for your review Kuroda-san, I have changed the logic to not use
already_locked and instead check if the locks are taken inside
UpdateSubscriptionRelState itself. I've tested this and this works
fine. If this logic is acceptable to the reviewers I can update the
other patches also in a similar way.
regards,
Ajin Cherian
Fujitsu Australia