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.
>> 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.
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 547fe4cce9..bf122f861a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2600,13 +2600,14 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
<entry><structfield>checksum_failures</structfield></entry>
<entry><type>bigint</type></entry>
<entry>Number of data page checksum failures detected in this
- database</entry>
+ database, or NULL is data checksums are not enabled.</entry>
</row>
<row>
<entry><structfield>checksum_last_failure</structfield></entry>
<entry><type>timestamp with time zone</type></entry>
<entry>Time at which the last data page checksum failure was detected in
- this database, or on a shared object.</entry>
+ this database (or on a shared object), or NULL is data checksums are not
+ enabled.</entry>
</row>
<row>
<entry><structfield>blk_read_time</structfield></entry>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 161bad6c90..566100d6df 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -817,7 +817,7 @@ CREATE VIEW pg_stat_database AS
D.oid AS datid,
D.datname AS datname,
CASE
- WHEN (D.oid = (0)::oid) THEN NULL::integer
+ WHEN (D.oid = (0)::oid) THEN 0
ELSE pg_stat_get_db_numbackends(D.oid)
END AS numbackends,
pg_stat_get_db_xact_commit(D.oid) AS xact_commit,
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 97f41fb46c..05240bfd14 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include "access/htup_details.h"
+#include "access/xlog.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "common/ip.h"
@@ -1526,6 +1527,9 @@ pg_stat_get_db_checksum_failures(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatDBEntry *dbentry;
+ if (!DataChecksumsEnabled())
+ PG_RETURN_NULL();
+
if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
result = 0;
else
@@ -1541,6 +1545,9 @@ pg_stat_get_db_checksum_last_failure(PG_FUNCTION_ARGS)
TimestampTz result;
PgStat_StatDBEntry *dbentry;
+ if (!DataChecksumsEnabled())
+ PG_RETURN_NULL();
+
if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
result = 0;
else
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 30973904c5..0c392e51e2 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1806,7 +1806,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints
pg_stat_database| SELECT d.oid AS datid,
d.datname,
CASE
- WHEN (d.oid = (0)::oid) THEN NULL::integer
+ WHEN (d.oid = (0)::oid) THEN 0
ELSE pg_stat_get_db_numbackends(d.oid)
END AS numbackends,
pg_stat_get_db_xact_commit(d.oid) AS xact_commit,
