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

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Inconsistency in determining the timestamp of the db statfile.
Date: 2020-09-10 07:33:43
Message-ID: CABUevExtTttBw4O_BQ-yAimvHZiFyK3Otfea8eKTMR=0+e_UJQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

> On Thu, Sep 10, 2020 at 11:52 AM Masahiko Sawada
> <masahiko(dot)sawada(at)2ndquadrant(dot)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/>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2020-09-10 07:40:15 Re: Proposals for making it easier to write correct bgworkers
Previous Message Georgios 2020-09-10 07:32:27 Re: v13: show extended stats target in \d