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

At Mon, 30 Jan 2023 14:24:31 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in
> On Mon, Jan 30, 2023 at 12:38 PM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > At Mon, 30 Jan 2023 11:56:33 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in
> > > #define TimestampTzPlusMilliseconds(tz,ms) ((tz) + ((ms) * (int64) 1000))
> >
> > Which can lead to overflow, which is practically harmless.
> >
>
> But here tz is always TimestampTz (which is int64), so do, we need to worry?

Sorry, I was putting an assuption that int were int64 here.

> > > the function defGetMinApplyDelay() sufficient to ensure that the
> > > 'delay' value stored in the catalog will always be lesser than
> > > INT_MAX?
> >
> > I'm concerned about cases where INT_MAX is wider than int32. If we
> > don't assume such cases, I'm fine with INT_MAX there.
> >
>
> I am not aware of such cases. Anyway, if any such case is discovered
> then we need to change the checks in defGetMinApplyDelay(), right? If
> so, then I think it is better to keep it as it is unless we know that
> this could be an issue on some platform.

I'm not sure. I think that int is generally thought that it is tied
with an integer type of any size. min_apply_delay is tightly bond
with a catalog column of int32 thus I thought that (PG_)INT32_MAX is
the right limit. So, as I expressed before, if we assume sizeof(int)
<= sizeof(int32), I' fine with using INT_MAX there.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center