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