On Tue, Feb 08, 2022 at 12:56:59PM +0900, Michael Paquier wrote: > Well, I can see that this is a second independent complain after a few > months. If you wish to keep this capability, wouldn't it be better to > add a "regress" mode to compute_query_id, where we would mask > automatically this information in the output of EXPLAIN but still run > the computation?
So, I have been looking at this problem, and I don't see a problem in doing something like the attached, where we add a "regress" mode to compute_query_id that is a synonym of "auto". Or, in short, we have the default of letting a module decide if a query ID can be computed or not, at the exception that we hide its result in EXPLAIN outputs. Julien, what do you think? FWIW, about your question of upthread, I have noticed the behavior while testing, but I know of some internal customers that enable pg_stat_statements and like doing tests on the PostgreSQL instance deployed this way, so that would break. They are not on 14 yet as far as I know. -- Michael
diff --git a/src/include/utils/queryjumble.h b/src/include/utils/queryjumble.h
index a4c277269e..c670662db2 100644
--- a/src/include/utils/queryjumble.h
+++ b/src/include/utils/queryjumble.h
@@ -57,7 +57,8 @@ enum ComputeQueryIdType
{
COMPUTE_QUERY_ID_OFF,
COMPUTE_QUERY_ID_ON,
- COMPUTE_QUERY_ID_AUTO
+ COMPUTE_QUERY_ID_AUTO,
+ COMPUTE_QUERY_ID_REGRESS
};
/* GUC parameters */
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index b970997c34..fae3926b42 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -604,7 +604,13 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
/* Create textual dump of plan tree */
ExplainPrintPlan(es, queryDesc);
- if (es->verbose && plannedstmt->queryId != UINT64CONST(0))
+ /*
+ * COMPUTE_QUERY_ID_REGRESS means COMPUTE_QUERY_ID_AUTO, but we don't
+ * show it up in any of the EXPLAIN plans to keep stable the results
+ * generated by any regression test suite.
+ */
+ if (es->verbose && plannedstmt->queryId != UINT64CONST(0) &&
+ compute_query_id != COMPUTE_QUERY_ID_REGRESS)
{
/*
* Output the queryid as an int64 rather than a uint64 so we match
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 01f373815e..7438df9109 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -412,6 +412,7 @@ static const struct config_enum_entry backslash_quote_options[] = {
*/
static const struct config_enum_entry compute_query_id_options[] = {
{"auto", COMPUTE_QUERY_ID_AUTO, false},
+ {"regress", COMPUTE_QUERY_ID_REGRESS, false},
{"on", COMPUTE_QUERY_ID_ON, false},
{"off", COMPUTE_QUERY_ID_OFF, false},
{"true", COMPUTE_QUERY_ID_ON, true},
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index e6f71c7582..a4612d9a8a 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1925,8 +1925,9 @@ create_database(const char *dbname)
"ALTER DATABASE \"%s\" SET lc_numeric TO 'C';"
"ALTER DATABASE \"%s\" SET lc_time TO 'C';"
"ALTER DATABASE \"%s\" SET bytea_output TO 'hex';"
+ "ALTER DATABASE \"%s\" SET compute_query_id TO 'regress';"
"ALTER DATABASE \"%s\" SET timezone_abbreviations TO 'Default';",
- dbname, dbname, dbname, dbname, dbname, dbname);
+ dbname, dbname, dbname, dbname, dbname, dbname, dbname);
psql_end_command(buf, "postgres");
/*
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d99bf38e67..036e6da680 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7934,9 +7934,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
method is not acceptable. In this case, in-core computation
must be always disabled.
Valid values are <literal>off</literal> (always disabled),
- <literal>on</literal> (always enabled) and <literal>auto</literal>,
+ <literal>on</literal> (always enabled), <literal>auto</literal>,
which lets modules such as <xref linkend="pgstatstatements"/>
- automatically enable it.
+ automatically enable it, and <literal>regress</literal> which
+ has the same effect as <literal>auto</literal>, except that the
+ query identifier is hidden in the <literal>EXPLAIN</literal> output.
The default is <literal>auto</literal>.
</para>
<note>
signature.asc
Description: PGP signature
