From 12a605ca84bf21439e4ae51cc3f3a891b3cb4989 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sat, 22 Feb 2020 21:17:10 -0600 Subject: [PATCH 1/7] Add GUC: explain_regress This changes the defaults for explain to: costs off, timing off, summary off. It'd be reasonable to use this for new regression tests which are not intended to be backpatched. --- contrib/postgres_fdw/postgres_fdw.c | 2 +- src/backend/commands/explain.c | 13 +++++++++++-- src/backend/utils/misc/guc_tables.c | 13 +++++++++++++ src/include/commands/explain.h | 2 ++ src/test/regress/expected/explain.out | 3 +++ src/test/regress/expected/inherit.out | 2 +- src/test/regress/expected/stats_ext.out | 2 +- src/test/regress/pg_regress.c | 3 ++- src/test/regress/sql/explain.sql | 4 ++++ src/test/regress/sql/inherit.sql | 2 +- src/test/regress/sql/stats_ext.sql | 2 +- 11 files changed, 40 insertions(+), 8 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 8d7500abfbd..3a4f56695b1 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -3138,7 +3138,7 @@ estimate_path_cost_size(PlannerInfo *root, * values, so don't request params_list. */ initStringInfo(&sql); - appendStringInfoString(&sql, "EXPLAIN "); + appendStringInfoString(&sql, "EXPLAIN (COSTS)"); deparseSelectStmtForRel(&sql, root, foreignrel, fdw_scan_tlist, remote_conds, pathkeys, fpextra ? fpextra->has_final_sort : false, diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index f86983c6601..373fde4d498 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -154,6 +154,7 @@ static void ExplainJSONLineEnding(ExplainState *es); static void ExplainYAMLLineStarting(ExplainState *es); static void escape_yaml(StringInfo buf, const char *str); +bool explain_regress = false; /* GUC */ /* @@ -172,6 +173,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, ListCell *lc; bool timing_set = false; bool summary_set = false; + bool costs_set = false; /* Parse options list. */ foreach(lc, stmt->options) @@ -183,7 +185,11 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, else if (strcmp(opt->defname, "verbose") == 0) es->verbose = defGetBoolean(opt); else if (strcmp(opt->defname, "costs") == 0) + { + /* Need to keep track if it was explicitly set to ON */ + costs_set = true; es->costs = defGetBoolean(opt); + } else if (strcmp(opt->defname, "buffers") == 0) es->buffers = defGetBoolean(opt); else if (strcmp(opt->defname, "wal") == 0) @@ -227,13 +233,16 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, parser_errposition(pstate, opt->location))); } + /* if the costs option was not set explicitly, set default value */ + es->costs = (costs_set) ? es->costs : es->costs && !explain_regress; + if (es->wal && !es->analyze) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("EXPLAIN option WAL requires ANALYZE"))); /* if the timing was not set explicitly, set default value */ - es->timing = (timing_set) ? es->timing : es->analyze; + es->timing = (timing_set) ? es->timing : es->analyze && !explain_regress; /* check that timing is used with EXPLAIN ANALYZE */ if (es->timing && !es->analyze) @@ -242,7 +251,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, errmsg("EXPLAIN option TIMING requires ANALYZE"))); /* if the summary was not set explicitly, set default value */ - es->summary = (summary_set) ? es->summary : es->analyze; + es->summary = (summary_set) ? es->summary : es->analyze && !explain_regress; query = castNode(Query, stmt->query); if (IsQueryIdEnabled()) diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 05ab087934c..1d34e6bdb7b 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -36,6 +36,7 @@ #include "catalog/namespace.h" #include "catalog/storage.h" #include "commands/async.h" +#include "commands/explain.h" #include "commands/tablespace.h" #include "commands/trigger.h" #include "commands/user.h" @@ -967,6 +968,18 @@ struct config_bool ConfigureNamesBool[] = true, NULL, NULL, NULL }, + + { + {"explain_regress", PGC_USERSET, DEVELOPER_OPTIONS, + gettext_noop("Change defaults of EXPLAIN to avoid unstable output."), + NULL, + GUC_NOT_IN_SAMPLE | GUC_EXPLAIN + }, + &explain_regress, + false, + NULL, NULL, NULL + }, + { {"geqo", PGC_USERSET, QUERY_TUNING_GEQO, gettext_noop("Enables genetic query optimization."), diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h index 9ebde089aed..912bc9484ef 100644 --- a/src/include/commands/explain.h +++ b/src/include/commands/explain.h @@ -61,6 +61,8 @@ typedef struct ExplainState ExplainWorkersState *workers_state; /* needed if parallel plan */ } ExplainState; +extern bool explain_regress; + /* Hook for plugins to get control in ExplainOneQuery() */ typedef void (*ExplainOneQuery_hook_type) (Query *query, int cursorOptions, diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out index 48620edbc2b..4abc5a346c6 100644 --- a/src/test/regress/expected/explain.out +++ b/src/test/regress/expected/explain.out @@ -8,6 +8,9 @@ -- To produce stable regression test output, it's usually necessary to -- ignore details such as exact costs or row counts. These filter -- functions replace changeable output details with fixed strings. +-- Output normal, user-facing details, not the sanitized version used for the +-- rest of the regression tests +set explain_regress = off; create function explain_filter(text) returns setof text language plpgsql as $$ diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 2d49e765de8..38fb5f94c6a 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -664,7 +664,7 @@ select tableoid::regclass::text as relname, parted_tab.* from parted_tab order b (3 rows) -- modifies partition key, but no rows will actually be updated -explain update parted_tab set a = 2 where false; +explain (costs on) update parted_tab set a = 2 where false; QUERY PLAN -------------------------------------------------------- Update on parted_tab (cost=0.00..0.00 rows=0 width=0) diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index a2bc409e06f..4b8f93ccf65 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -14,7 +14,7 @@ declare first_row bool := true; begin for ln in - execute format('explain analyze %s', $1) + execute format('explain (analyze, costs on) %s', $1) loop if first_row then first_row := false; diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index dda076847a3..de1a7f7057b 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -625,7 +625,7 @@ initialize_environment(void) * user's ability to set other variables through that. */ { - const char *my_pgoptions = "-c intervalstyle=postgres_verbose"; + const char *my_pgoptions = "-c intervalstyle=postgres_verbose -c explain_regress=on"; const char *old_pgoptions = getenv("PGOPTIONS"); char *new_pgoptions; @@ -2288,6 +2288,7 @@ regression_main(int argc, char *argv[], fputs("log_lock_waits = on\n", pg_conf); fputs("log_temp_files = 128kB\n", pg_conf); fputs("max_prepared_transactions = 2\n", pg_conf); + // fputs("explain_regress = yes\n", pg_conf); for (sl = temp_configs; sl != NULL; sl = sl->next) { diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql index ae3f7a308d7..dbb3799d5e2 100644 --- a/src/test/regress/sql/explain.sql +++ b/src/test/regress/sql/explain.sql @@ -10,6 +10,10 @@ -- ignore details such as exact costs or row counts. These filter -- functions replace changeable output details with fixed strings. +-- Output normal, user-facing details, not the sanitized version used for the +-- rest of the regression tests +set explain_regress = off; + create function explain_filter(text) returns setof text language plpgsql as $$ diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 195aedb5ff5..868ee58b803 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -169,7 +169,7 @@ where parted_tab.a = ss.a; select tableoid::regclass::text as relname, parted_tab.* from parted_tab order by 1,2; -- modifies partition key, but no rows will actually be updated -explain update parted_tab set a = 2 where false; +explain (costs on) update parted_tab set a = 2 where false; drop table parted_tab; diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index 19417561bd6..5bd6b9a41ab 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -16,7 +16,7 @@ declare first_row bool := true; begin for ln in - execute format('explain analyze %s', $1) + execute format('explain (analyze, costs on) %s', $1) loop if first_row then first_row := false; -- 2.25.1