Re: Inconsistency in determining the timestamp of the db statfile.

On Thu, Sep 10, 2020 at 9:05 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

> On Thu, Sep 10, 2020 at 11:52 AM Masahiko Sawada
> <masahiko.sawada@2ndquadrant.com> wrote:
> >
> >
> > Regarding the v2 patch, I think we should return false in the
> > following case too:
> >
> >             default:
> >                 ereport(pgStatRunningInCollector ? LOG : WARNING,
> >                         (errmsg("corrupted statistics file \"%s\"",
> >                                 statfile)));
> >                 goto done;
> >
>
> makes sense, attached find the updated patch.
>

As a minor nitpick, technically, I think the comment change is wrong,
because it says that the caller *must* rely on the timestamp, which it of
course doesn't. I think a more proper one is "The caller must not rely on
the timestamp in case the function returns false" or "The caller must only
rely on the timestamp if the function returns true".

+1 on the code parts.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>