Re: Avoid resetting subscription two_phase state to pending when already enabled

[Zhijie Hou]

Hi,

On Mon, Nov  2, 2026 at 10:04 AM Peter Smith wrote:
IMO, the nested checking of opts.twophase in the ternary makes the
patch code unnecessarily hard to follow across combinations.
My suggestion is to implement it using more verbose, but easier-to-read code:

      /* Change system catalog accordingly */
if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_DISABLED)
{
     if (opts.twophase)
     {
         /* enable two_phase */
         values[Anum_pg_subscription_subtwophasestate - 1] =
             CharGetDatum(LOGICALREP_TWOPHASE_STATE_PENDING);
         replaces[Anum_pg_subscription_subtwophasestate - 1] = true;
     }
}
else if (!opts.twophase)
{
     /* disable two_phase */
     values[Anum_pg_subscription_subtwophasestate - 1] =
         CharGetDatum(LOGICALREP_TWOPHASE_STATE_DISABLED);
     replaces[Anum_pg_subscription_subtwophasestate - 1] = true;
}

~

Thank for the suggestion, but I personally don't prefer the longer, 
nested style, as I don't find it clearly easier to read. So I would keep 
the current version for now and see what the committer thinks.

--
Best Regards,
Zhijie Hou