On Wed, Sep 9, 2020 at 9:37 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > On 2020/09/09 22:57, Magnus Hagander wrote: > > On Wed, Sep 9, 2020 at 3:56 PM Tomas Vondra <tomas.von...@2ndquadrant.com > > <mailto:tomas.von...@2ndquadrant.com>> wrote: > > > > On Wed, Sep 09, 2020 at 03:53:40PM +0530, Amit Kapila wrote: > > >On Wed, Sep 9, 2020 at 3:15 PM Magnus Hagander <mag...@hagander.net > > <mailto:mag...@hagander.net>> wrote: > > >> > > >> On Wed, Sep 9, 2020 at 5:04 AM Amit Kapila <amit.kapil...@gmail.com > > <mailto:amit.kapil...@gmail.com>> wrote: > > >>> > > >> > > >> Though in fact the one inconsistent place in the code now is that > > if it is corrupt in the db entry part of the file it returns true and the > > global timestamp, which I would argue is perhaps incorrect and it should > > return false. > > >> > > > > > >Yeah, this is exactly the case I was pointing out where we return true > > >before the patch, basically the code below: > > >case 'D': > > >if (fread(&dbentry, 1, offsetof(PgStat_StatDBEntry, tables), > > > fpin) != offsetof(PgStat_StatDBEntry, tables)) > > >{ > > >ereport(pgStatRunningInCollector ? LOG : WARNING, > > >(errmsg("corrupted statistics file \"%s\"", > > >statfile))); > > >goto done; > > >} > > > > > >done: > > >FreeFile(fpin); > > >return true; > > > > > >Now, if we decide to return 'false' here, then surely there is no > > >argument and we should return false in other cases as well. Basically, > > >I think we should be consistent in handling the corrupt file case. > > > > > > > FWIW I do agree with this - we should return false here, to make it > > return false like in the other data corruption cases. AFAICS that's the > > only inconsistency here. > > > > > > +1, I think that's the place to fix, rather than reversing all the other > > places. > > +1 as I suggested upthread! >
Please find the patch attached based on the above discussion. I have slightly adjusted the comments. -- With Regards, Amit Kapila.
v2-0001-Fix-inconsistency-in-determining-the-timestamp-of.patch
Description: Binary data
