Expand assertion check for query ID reporting in executor

As formulated, the assertion added in the executor by 24f520594809 to
check that a query ID is set had two problems:
- track_activities may be disabled while compute_query_id is enabled,
causing the query ID to not be reported to pg_stat_activity.
- debug_query_string may not be set in some context.  The only path
where this would matter is visibly autovacuum, should parallel workers
be enabled there at some point.  This is not the case currently.

There was no test showing the interactions between the query ID and
track_activities, so let's add one based on a scan of pg_stat_activity.
This assertion is still an experimentation at this stage, but let's see
if this shows more paths where query IDs are not properly set while they
should.

Discussion: https://postgr.es/m/[email protected]

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 728cdee480ba81f2b7e1ae38c67850c6d92d45df..b5fbd8af97ec059fe6baeec71d37f2fee686e80d 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -71,6 +71,18 @@ ExecutorEnd_hook_type ExecutorEnd_hook = NULL;
 /* Hook for plugin to get control in ExecCheckPermissions() */
 ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
 
+/*
+ * Check that the query ID is set, which is something that happens only
+ * if compute_query_id is enabled (or a module forced it), if track_activities
+ * is enabled, and if a client provided a query string to map with the query
+ * ID computed from it.
+ */
+#define EXEC_CHECK_QUERY_ID \
+do { \
+   Assert(!IsQueryIdEnabled() || !pgstat_track_activities ||       \
+          !debug_query_string || pgstat_get_my_query_id() != 0);   \
+} while(0)
+
 /* decls for local routines only used within this module */
 static void InitPlan(QueryDesc *queryDesc, int eflags);
 static void CheckValidRowMarkRel(Relation rel, RowMarkType markType);
@@ -297,7 +309,7 @@ ExecutorRun(QueryDesc *queryDesc,
            bool execute_once)
 {
    /* If enabled, the query ID should be set. */
-   Assert(!IsQueryIdEnabled() || pgstat_get_my_query_id() != 0);
+   EXEC_CHECK_QUERY_ID;
 
    if (ExecutorRun_hook)
        (*ExecutorRun_hook) (queryDesc, direction, count, execute_once);
@@ -408,7 +420,7 @@ void
 ExecutorFinish(QueryDesc *queryDesc)
 {
    /* If enabled, the query ID should be set. */
-   Assert(!IsQueryIdEnabled() || pgstat_get_my_query_id() != 0);
+   EXEC_CHECK_QUERY_ID;
 
    if (ExecutorFinish_hook)
        (*ExecutorFinish_hook) (queryDesc);
@@ -471,7 +483,7 @@ void
 ExecutorEnd(QueryDesc *queryDesc)
 {
    /* If enabled, the query ID should be set. */
-   Assert(!IsQueryIdEnabled() || pgstat_get_my_query_id() != 0);
+   EXEC_CHECK_QUERY_ID;
 
    if (ExecutorEnd_hook)
        (*ExecutorEnd_hook) (queryDesc);

diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 455b6d6c0ce54532002a1cda2682afdabb96b25c..14edb42704af9c1d5384ae5fa781a4074d4ff23e 100644 (file)
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -824,6 +824,27 @@ set default_with_oids to f;
 -- Should not allow to set it to true.
 set default_with_oids to t;
 ERROR:  tables declared WITH OIDS are not supported
+-- Test that disabling track_activities disables query ID reporting in
+-- pg_stat_activity.
+SET compute_query_id = on;
+SET track_activities = on;
+SELECT query_id IS NOT NULL AS qid_set FROM pg_stat_activity
+  WHERE pid = pg_backend_pid();
+ qid_set 
+---------
+ t
+(1 row)
+
+SET track_activities = off;
+SELECT query_id IS NOT NULL AS qid_set FROM pg_stat_activity
+  WHERE pid = pg_backend_pid();
+ qid_set 
+---------
+ f
+(1 row)
+
+RESET track_activities;
+RESET compute_query_id;
 -- Test GUC categories and flag patterns
 SELECT pg_settings_get_flags(NULL);
  pg_settings_get_flags 

diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index dc79761955d0c0a4f4037ce998e6a647a778ed5a..2be7ab294038c0de890f6163a53ddae6974bc11f 100644 (file)
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -319,6 +319,18 @@ set default_with_oids to f;
 -- Should not allow to set it to true.
 set default_with_oids to t;
 
+-- Test that disabling track_activities disables query ID reporting in
+-- pg_stat_activity.
+SET compute_query_id = on;
+SET track_activities = on;
+SELECT query_id IS NOT NULL AS qid_set FROM pg_stat_activity
+  WHERE pid = pg_backend_pid();
+SET track_activities = off;
+SELECT query_id IS NOT NULL AS qid_set FROM pg_stat_activity
+  WHERE pid = pg_backend_pid();
+RESET track_activities;
+RESET compute_query_id;
+
 -- Test GUC categories and flag patterns
 SELECT pg_settings_get_flags(NULL);
 SELECT pg_settings_get_flags('does_not_exist');