Re: Checksum errors in pg_stat_database

Sorry for late reply,

On Sun, Apr 14, 2019 at 7:12 PM Magnus Hagander <magnus@hagander.net> wrote:
>
> On Sat, Apr 13, 2019 at 8:46 PM Robert Treat <rob@xzilla.net> wrote:
>>
>> On Fri, Apr 12, 2019 at 8:18 AM Magnus Hagander <magnus@hagander.net> wrote:
>> ISTM the argument here is go with zero since you have zero connections
>> vs go with null since you can't actually connect, so it doesn't make
>> sense. (There is a third argument about making it -1 since you can't
>> connect, but that breaks sum(numbackends) so it's easily dismissed.) I
>> think I would have gone for 0 personally, but what ended up surprising
>> me was that a bunch of other stuff like xact_commit show zero when
>> AFAICT the above reasoning would apply the same to those columns.
>> (unless there is a way to commit a transaction in the global objects
>> that I don't know about).
>
>
> That's a good point. I mean, you can commit a transaction that involves changes of global objects, but it counts in
thedatabase that you were conneced to.
 
>
> We should probably at least make it consistent and make it NULL in all or 0 in all.
>
> I'm -1 for using -1 (!), for the very reason that you mention. But either changing the numbackends to 0, or the
othersto NULL would work for consistency. I'm leaning towards the 0 as well.
 

+1 for 0 :)  Especially since it's less code in the view.

>> What originally got me looking at this was the idea of returning -1
>> (or maybe null) for checksum failures for cases when checksums are not
>> enabled. This seems a little more complicated to set up, but seems
>> like it might ward off people thinking they are safe due to no
>> checksum error reports when they actually aren't.
>
>
> NULL seems like the reasonable thing to return there. I'm not sure what you're referring to with a little more
complicatedto set up, thought? Do you mean somehow for the end user?
 
>
> Code-wise it seems it should be simple -- just do an "if checksums disabled then return null"  in the two functions.

That's indeed a good point!  Lack of checksum error is distinct from
checksums not activated and we should make it obvious.

I don't know if that counts as an open item, but I attach a patch for
all points discussed here.