On Mon, Jan 30, 2023 at 12:38 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Mon, 30 Jan 2023 11:56:33 +0530, Amit Kapila <amit.kapil...@gmail.com> > wrote in > > > > > > 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 > > Where recovery_min_apply_delay is used. It is allowed to be set up to > INT_MAX but it is used as: > > > delayUntil = TimestampTzPlusMilliseconds(xtime, > > recovery_min_apply_delay); > > Where the macro is defined as: > > > #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? > > 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. -- With Regards, Amit Kapila.
