Re: Time delayed LR (WAS Re: logical replication restrictions)

On Mon, Jan 30, 2023 at 9:43 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Mon, 30 Jan 2023 08:51:05 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in
> > On Mon, Jan 30, 2023 at 8:32 AM Kyotaro Horiguchi
> > <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > >
> > > At Sat, 28 Jan 2023 04:28:29 +0000, "Takamichi Osumi (Fujitsu)" <osumi(dot)takamichi(at)fujitsu(dot)com> wrote in
> > > > On Friday, January 27, 2023 8:00 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > > So, you have changed min_apply_delay from int64 to int32, but you haven't
> > > > > mentioned the reason for the same? We use 'int' for the similar parameter
> > > > > recovery_min_apply_delay, so, ideally, it makes sense but still better to tell your
> > > > > reason explicitly.
> > > > Yes. It's because I thought I need to make this feature consistent with the recovery_min_apply_delay.
> > > > This feature handles the range same as the recovery_min_apply delay from 0 to INT_MAX now
> > > > so should be adjusted to match it.
> > >
> > > INT_MAX can stick out of int32 on some platforms. (I'm not sure where
> > > that actually happens, though.) We can use PG_INT32_MAX instead.
> > >
> >
> > But in other integer GUCs including recovery_min_apply_delay, we use
> > INT_MAX, so not sure if it is a good idea to do something different
> > here.
>
> The GUC is not stored in a catalog, but.. oh... it is multiplied by
> 1000.

Which part of the patch you are referring to here? Isn't the check in
the function defGetMinApplyDelay() sufficient to ensure that the
'delay' value stored in the catalog will always be lesser than
INT_MAX?

--
With Regards,
Amit Kapila.