Allow compute_query_id to be set to 'auto' and make it default
authorAlvaro Herrera
Sat, 15 May 2021 18:13:09 +0000 (14:13 -0400)
committerAlvaro Herrera
Sat, 15 May 2021 18:13:09 +0000 (14:13 -0400)
Allowing only on/off meant that all either all existing configuration
guides would become obsolete if we disabled it by default, or that we
would have to accept a performance loss in the default config if we
enabled it by default.  By allowing 'auto' as a middle ground, the
performance cost is only paid by those who enable pg_stat_statements and
similar modules.

I only edited the release notes to comment-out a paragraph that is now
factually wrong; further edits are probably needed to describe the
related change in more detail.

Author: Julien Rouhaud 
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/20210513002623.eugftm4nk2lvvks3@nol

14 files changed:
contrib/pg_stat_statements/pg_stat_statements.c
contrib/pg_stat_statements/pg_stat_statements.conf
doc/src/sgml/config.sgml
doc/src/sgml/pgstatstatements.sgml
doc/src/sgml/release-14.sgml
src/backend/commands/explain.c
src/backend/parser/analyze.c
src/backend/postmaster/postmaster.c
src/backend/tcop/postgres.c
src/backend/utils/misc/guc.c
src/backend/utils/misc/postgresql.conf.sample
src/backend/utils/misc/queryjumble.c
src/include/utils/guc.h
src/include/utils/queryjumble.h

index a85f9628013f2d8a3169e74d4043cccdc2759ffa..09433c8c96c75af4bff8e1fd34ad5c8cce57192e 100644 (file)
@@ -369,6 +369,12 @@ _PG_init(void)
    if (!process_shared_preload_libraries_in_progress)
        return;
 
+   /*
+    * Inform the postmaster that we want to enable query_id calculation if
+    * compute_query_id is set to auto.
+    */
+   EnableQueryId();
+
    /*
     * Define (or redefine) custom GUC variables.
     */
index e47b26040ffcd4dffbcdb52bbd1557631993bfcb..13346e28078358a9d2cc2744bb7866b9ad09ead4 100644 (file)
@@ -1,2 +1 @@
 shared_preload_libraries = 'pg_stat_statements'
-compute_query_id = on
index 45bd1f1b7e3bc135661dd4307091d89dbd6c0f5c..7e32b0686c6ae91598e9405f0568ec7e85543cd7 100644 (file)
@@ -7627,7 +7627,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
      
 
      
-      compute_query_id (boolean)
+      compute_query_id (enum)
       
        compute_query_id configuration parameter
       
@@ -7643,7 +7643,12 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         identifier to be computed.  Note that an external module can
         alternatively be used if the in-core query identifier computation
         method is not acceptable.  In this case, in-core computation
-        must be disabled.  The default is off.
+        must be always disabled.
+        Valid values are off (always disabled),
+        on (always enabled) and auto,
+        which lets modules such as 
+        automatically enable it.
+        The default is auto.
        
        
         
index bc2b6038ee851995e3e0672891e2755e3a07cf24..aa332d8cc2264d30a08da647c9dd63e985fcfcf3 100644 (file)
    in
   postgresql.conf, because it requires additional shared memory.
   This means that a server restart is needed to add or remove the module.
+  In addition, query identifier calculation must be enabled in order for the
+  module to be active, which is done automatically if 
+  is set to auto or on, or any third-party
+  module that calculates query identifiers is loaded.
  
 
  
-  The module will not track statistics unless query
-  identifiers are calculated.  This can be done by enabling 
-  linkend="guc-compute-query-id"/> or using a third-party module that
-  computes its own query identifiers.  Note that all statistics tracked
-  by this module must be reset if the query identifier method is changed.
-
-   When pg_stat_statements is loaded, it tracks
+   When pg_stat_statements is active, it tracks
    statistics across all databases of the server.  To access and manipulate
    these statistics, the module provides views
    pg_stat_statements and
index c4df401c686296e7bacb8d885b9f6b02ea48d645..bad12860924e8430cc96065db4c7b206fc12518b 100644 (file)
@@ -3181,10 +3181,12 @@ Author: Bruce Momjian 
 Move query hash computation from pg_stat_statements to the core server (Julien Rouhaud)
 
 
+
 
 
 
index 1202bf85a367a86c0d3952a1b1c37e806c8c092f..9a60865d19111d752e287d48a46cbce4ac3c3eee 100644 (file)
@@ -245,7 +245,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt,
    es->summary = (summary_set) ? es->summary : es->analyze;
 
    query = castNode(Query, stmt->query);
-   if (compute_query_id)
+   if (IsQueryIdEnabled())
        jstate = JumbleQuery(query, pstate->p_sourcetext);
 
    if (post_parse_analyze_hook)
index 168198acd143c4013a524933a633835a3d6eb225..201b88d1adb1d459844625c06c6182fc27eabf54 100644 (file)
@@ -124,7 +124,7 @@ parse_analyze(RawStmt *parseTree, const char *sourceText,
 
    query = transformTopLevelStmt(pstate, parseTree);
 
-   if (compute_query_id)
+   if (IsQueryIdEnabled())
        jstate = JumbleQuery(query, sourceText);
 
    if (post_parse_analyze_hook)
@@ -163,7 +163,7 @@ parse_analyze_varparams(RawStmt *parseTree, const char *sourceText,
    /* make sure all is well with parameter types */
    check_variable_parameters(pstate, query);
 
-   if (compute_query_id)
+   if (IsQueryIdEnabled())
        jstate = JumbleQuery(query, sourceText);
 
    if (post_parse_analyze_hook)
index 6833f0f7f2dbcc33ef9d5e16e3a8a6f1cc980211..9ca1095f47feb77489ecb7208931c7594da223c6 100644 (file)
@@ -521,6 +521,7 @@ typedef struct
    pg_time_t   first_syslogger_file_time;
    bool        redirection_done;
    bool        IsBinaryUpgrade;
+   bool        auto_query_id_enabled;
    int         max_safe_fds;
    int         MaxBackends;
 #ifdef WIN32
@@ -6168,6 +6169,7 @@ save_backend_variables(BackendParameters *param, Port *port,
 
    param->redirection_done = redirection_done;
    param->IsBinaryUpgrade = IsBinaryUpgrade;
+   param->auto_query_id_enabled = auto_query_id_enabled;
    param->max_safe_fds = max_safe_fds;
 
    param->MaxBackends = MaxBackends;
@@ -6401,6 +6403,7 @@ restore_backend_variables(BackendParameters *param, Port *port)
 
    redirection_done = param->redirection_done;
    IsBinaryUpgrade = param->IsBinaryUpgrade;
+   auto_query_id_enabled = param->auto_query_id_enabled;
    max_safe_fds = param->max_safe_fds;
 
    MaxBackends = param->MaxBackends;
index dd2ade7bb652c02a23913d2875ae332a363ec066..8cea10c90191b11c3c1c9317cdec2f79c5b58ace 100644 (file)
@@ -704,7 +704,7 @@ pg_analyze_and_rewrite_params(RawStmt *parsetree,
 
    query = transformTopLevelStmt(pstate, parsetree);
 
-   if (compute_query_id)
+   if (IsQueryIdEnabled())
        jstate = JumbleQuery(query, query_string);
 
    if (post_parse_analyze_hook)
index eb7f7181e43df51f4664b7854a2e38f1b65e301f..ee731044b63951b52b7d0e68e36f0e72857cffd5 100644 (file)
 #include "utils/plancache.h"
 #include "utils/portal.h"
 #include "utils/ps_status.h"
+#include "utils/queryjumble.h"
 #include "utils/rls.h"
 #include "utils/snapmgr.h"
 #include "utils/tzparser.h"
@@ -402,6 +403,23 @@ static const struct config_enum_entry backslash_quote_options[] = {
    {NULL, 0, false}
 };
 
+/*
+ * Although only "on", "off", and "auto" are documented, we accept
+ * all the likely variants of "on" and "off".
+ */
+static const struct config_enum_entry compute_query_id_options[] = {
+   {"auto", COMPUTE_QUERY_ID_AUTO, false},
+   {"on", COMPUTE_QUERY_ID_ON, false},
+   {"off", COMPUTE_QUERY_ID_OFF, false},
+   {"true", COMPUTE_QUERY_ID_ON, true},
+   {"false", COMPUTE_QUERY_ID_OFF, true},
+   {"yes", COMPUTE_QUERY_ID_ON, true},
+   {"no", COMPUTE_QUERY_ID_OFF, true},
+   {"1", COMPUTE_QUERY_ID_ON, true},
+   {"0", COMPUTE_QUERY_ID_OFF, true},
+   {NULL, 0, false}
+};
+
 /*
  * Although only "on", "off", and "partition" are documented, we
  * accept all the likely variants of "on" and "off".
@@ -534,7 +552,6 @@ extern const struct config_enum_entry dynamic_shared_memory_options[];
 /*
  * GUC option variables that are exported from this module
  */
-bool       compute_query_id = false;
 bool       log_duration = false;
 bool       Debug_print_plan = false;
 bool       Debug_print_parse = false;
@@ -1441,15 +1458,6 @@ static struct config_bool ConfigureNamesBool[] =
        true,
        NULL, NULL, NULL
    },
-   {
-       {"compute_query_id", PGC_SUSET, STATS_MONITORING,
-           gettext_noop("Compute query identifiers."),
-           NULL
-       },
-       &compute_query_id,
-       false,
-       NULL, NULL, NULL
-   },
    {
        {"log_parser_stats", PGC_SUSET, STATS_MONITORING,
            gettext_noop("Writes parser performance statistics to the server log."),
@@ -4619,6 +4627,16 @@ static struct config_enum ConfigureNamesEnum[] =
        NULL, NULL, NULL
    },
 
+   {
+       {"compute_query_id", PGC_SUSET, STATS_MONITORING,
+           gettext_noop("Compute query identifiers."),
+           NULL
+       },
+       &compute_query_id,
+       COMPUTE_QUERY_ID_AUTO, compute_query_id_options,
+       NULL, NULL, NULL
+   },
+
    {
        {"constraint_exclusion", PGC_USERSET, QUERY_TUNING_OTHER,
            gettext_noop("Enables the planner to use constraints to optimize queries."),
index efde01ee566db4fc732946e244e28bb2f7f6aaf9..6e36e4c2eff6de673353637ba5e078a3dcb85069 100644 (file)
 
 # - Monitoring -
 
-#compute_query_id = off
+#compute_query_id = auto
 #log_statement_stats = off
 #log_parser_stats = off
 #log_planner_stats = off
index f004a9ce8cde1ecf0a9f54442b0d1be556314d6d..9f2cd1f127691af163771ce87943d27b6e46ad8a 100644 (file)
 
 #define JUMBLE_SIZE                1024    /* query serialization buffer size */
 
+/* GUC parameters */
+int            compute_query_id = COMPUTE_QUERY_ID_AUTO;
+
+/* True when compute_query_id is ON, or AUTO and a module requests them */
+bool       query_id_enabled = false;
+
 static uint64 compute_utility_query_id(const char *str, int query_location, int query_len);
 static void AppendJumble(JumbleState *jstate,
                         const unsigned char *item, Size size);
@@ -96,6 +102,8 @@ JumbleQuery(Query *query, const char *querytext)
 {
    JumbleState *jstate = NULL;
 
+   Assert(IsQueryIdEnabled());
+
    if (query->utilityStmt)
    {
        query->queryId = compute_utility_query_id(querytext,
@@ -132,6 +140,19 @@ JumbleQuery(Query *query, const char *querytext)
    return jstate;
 }
 
+/*
+ * Enables query identifier computation.
+ *
+ * Third-party plugins can use this function to inform core that they require
+ * a query identifier to be computed.
+ */
+void
+EnableQueryId(void)
+{
+   if (compute_query_id != COMPUTE_QUERY_ID_OFF)
+       query_id_enabled = true;
+}
+
 /*
  * Compute a query identifier for the given utility query string.
  */
index 24a5d9d3fb283b07017d148fa28d488def985053..a7c3a4958e064b2b72ccc6ecb6a33344831509b6 100644 (file)
@@ -247,7 +247,6 @@ extern bool log_btree_build_stats;
 extern PGDLLIMPORT bool check_function_bodies;
 extern bool session_auth_is_superuser;
 
-extern bool compute_query_id;
 extern bool log_duration;
 extern int log_parameter_max_length;
 extern int log_parameter_max_length_on_error;
index 83ba7339faee8eaadc0aedec8af5112f601d64c5..1f4d062babd273da59d99e2b8d55740fb577df6e 100644 (file)
@@ -52,7 +52,36 @@ typedef struct JumbleState
    int         highest_extern_param_id;
 } JumbleState;
 
-const char *CleanQuerytext(const char *query, int *location, int *len);
-JumbleState *JumbleQuery(Query *query, const char *querytext);
+/* Values for the compute_query_id GUC */
+typedef enum
+{
+   COMPUTE_QUERY_ID_OFF,
+   COMPUTE_QUERY_ID_ON,
+   COMPUTE_QUERY_ID_AUTO
+} ComputeQueryIdType;
+
+/* GUC parameters */
+extern int compute_query_id;
+
+
+extern const char *CleanQuerytext(const char *query, int *location, int *len);
+extern JumbleState *JumbleQuery(Query *query, const char *querytext);
+extern void EnableQueryId(void);
+
+extern bool query_id_enabled;
+
+/*
+ * Returns whether query identifier computation has been enabled, either
+ * directly in the GUC or by a module when the setting is 'auto'.
+ */
+static inline bool
+IsQueryIdEnabled(void)
+{
+   if (compute_query_id == COMPUTE_QUERY_ID_OFF)
+       return false;
+   if (compute_query_id == COMPUTE_QUERY_ID_ON)
+       return true;
+   return query_id_enabled;
+}
 
 #endif                         /* QUERYJUMBLE_H */