Allow compute_query_id to be set to 'auto' and make it default

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 <rjuju123@gmail.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20210513002623.eugftm4nk2lvvks3@nol

src/backend/commands/explain.c

@@ -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)

src/backend/parser/analyze.c

@@ -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)

src/backend/postmaster/postmaster.c

@@ -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;

src/backend/tcop/postgres.c

@@ -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)

src/backend/utils/misc/guc.c

@@ -101,6 +101,7 @@
 #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."),

src/backend/utils/misc/postgresql.conf.sample

@@ -604,7 +604,7 @@
 
 # - Monitoring -
 
-#compute_query_id = off
+#compute_query_id = auto
 #log_statement_stats = off
 #log_parser_stats = off
 #log_planner_stats = off

src/backend/utils/misc/queryjumble.c

@@ -39,6 +39,12 @@
 
 #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.
  */

src/include/utils/guc.h

@@ -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;

src/include/utils/queryjumble.h

@@ -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 */