On Thu, Apr 08, 2021 at 11:36:48PM +1200, Thomas Munro wrote:
> Hi Julien, Bruce,
>
> A warning appears on 32 bit systems:
>
> In file included from pgstatfuncs.c:15:
> pgstatfuncs.c: In function 'pg_stat_get_activity':
> ../../../../src/include/postgres.h:593:29: warning: cast to pointer
> from integer of different size [-Wint-to-pointer-cast]
> 593 | #define DatumGetPointer(X) ((Pointer) (X))
> | ^
> ../../../../src/include/postgres.h:678:42: note: in expansion of macro
> 'DatumGetPointer'
> 678 | #define DatumGetUInt64(X) (* ((uint64 *) DatumGetPointer(X)))
> | ^~~~~~~~~~~~~~~
> pgstatfuncs.c:920:18: note: in expansion of macro 'DatumGetUInt64'
> 920 | values[29] = DatumGetUInt64(beentry->st_queryid);
> | ^~~~~~~~~~~~~~
Wow, that's really embarrassing :(
> Hmm, maybe this should be UInt64GetDatum()?
Yes definitely. I'm attaching the previous patch for force_parallel_mode to
not forget it + a new one for this issue.
>From d74523dfb76e7583c27166ec10d72670654c3b7b Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouh...@free.fr>
Date: Thu, 8 Apr 2021 13:59:43 +0800
Subject: [PATCH v2 1/2] Ignore parallel workers in pg_stat_statements.
Oversight in 4f0b0966c8 which exposed queryid in parallel workers. Counters
are aggregated by the main backend process so parallel workers would report
duplicated activity, and could also report activity for the wrong entry as they
are only aware of the top level queryid.
Author: Julien Rouhaud
Reported-by: Andres Freund
Discussion: https://postgr.es/m/20210408051735.lfbdzun5zdlax...@alap3.anarazel.de
---
contrib/pg_stat_statements/pg_stat_statements.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index fc2677643b..dbd0d41d88 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -47,6 +47,7 @@
#include <sys/stat.h>
#include <unistd.h>
+#include "access/parallel.h"
#include "catalog/pg_authid.h"
#include "common/hashfn.h"
#include "executor/instrument.h"
@@ -278,8 +279,9 @@ static bool pgss_save; /* whether to save stats across shutdown */
#define pgss_enabled(level) \
+ (!IsParallelWorker() && \
(pgss_track == PGSS_TRACK_ALL || \
- (pgss_track == PGSS_TRACK_TOP && (level) == 0))
+ (pgss_track == PGSS_TRACK_TOP && (level) == 0)))
#define record_gc_qtexts() \
do { \
--
2.30.1
>From 61ff6d226761fcc8f2a28fe8e313382d1d46f098 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouh...@free.fr>
Date: Thu, 8 Apr 2021 20:05:14 +0800
Subject: [PATCH v2 2/2] Fix thinko in pg_stat_get_activity when retrieving the
queryid.
---
src/backend/utils/adt/pgstatfuncs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9fa4a93162..182b15e3f2 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -917,7 +917,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
if (beentry->st_queryid == 0)
nulls[29] = true;
else
- values[29] = DatumGetUInt64(beentry->st_queryid);
+ values[29] = UInt64GetDatum(beentry->st_queryid);
}
else
{
--
2.30.1
