On Tue, Apr 16, 2019 at 5:39 PM Robert Treat <r...@xzilla.net> wrote:
> On Mon, Apr 15, 2019 at 3:32 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > > > > Sorry for late reply, > > > > On Sun, Apr 14, 2019 at 7:12 PM Magnus Hagander <mag...@hagander.net> > wrote: > > > > > > On Sat, Apr 13, 2019 at 8:46 PM Robert Treat <r...@xzilla.net> wrote: > > >> > > >> On Fri, Apr 12, 2019 at 8:18 AM Magnus Hagander <mag...@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 the database 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 others to 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. > > > > +1 for 0 > > > >> 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 complicated to 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. > > ISTM we should mention shared objects in both places in the docs, and > want "NULL if data checksums" rather than "NULL is data checksums". > Attaching slightly modified patch with those changes, but otherwise > LGTM. > Interestingly enough, that patch comes out as corrupt. I have no idea why though :) v1 is fine. So I tried merging back your changes into it, and then pushing. Please doublecheck I didn't miss something :) -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
