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".