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

On Wed, Sep 9, 2020 at 3:15 PM Magnus Hagander <magnus@hagander.net> wrote:
>
> On Wed, Sep 9, 2020 at 5:04 AM Amit Kapila <amit.kapila16@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.

-- 
With Regards,
Amit Kapila.