Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3ec20c7

Browse files
committed
Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.
In non-TEXT output formats, the "Settings" field should appear whenrequested, even if it would be empty.Also, get rid of the premature optimization of counting all theGUC_EXPLAIN variables at startup. Since there was no provision foradjusting that count later, all it'd take would be some extension markinga parameter as GUC_EXPLAIN to risk an assertion failure or memory stomp.We could make get_explain_guc_options() count those variables on-the-fly,or dynamically resize its array ... but TBH I do not think that making atransient array of pointers a bit smaller is worth any extra complication,especially when you consider all the other transient space EXPLAIN eats.So just allocate that array at the max possible size.In HEAD, also add some regression test coverage for this feature.Because of the memory-stomp hazard, back-patch to v12 where thisfeature was added.Discussion:https://postgr.es/m/19416.1580069629@sss.pgh.pa.us
1 parentf37ff03 commit3ec20c7

File tree

4 files changed

+54
-50
lines changed

4 files changed

+54
-50
lines changed

‎src/backend/commands/explain.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -626,17 +626,11 @@ ExplainPrintSettings(ExplainState *es)
626626
/* request an array of relevant settings */
627627
gucs=get_explain_guc_options(&num);
628628

629-
/* also bail out of there are no options */
630-
if (!num)
631-
return;
632-
633629
if (es->format!=EXPLAIN_FORMAT_TEXT)
634630
{
635-
inti;
636-
637631
ExplainOpenGroup("Settings","Settings", true,es);
638632

639-
for (i=0;i<num;i++)
633+
for (inti=0;i<num;i++)
640634
{
641635
char*setting;
642636
structconfig_generic*conf=gucs[i];
@@ -650,12 +644,15 @@ ExplainPrintSettings(ExplainState *es)
650644
}
651645
else
652646
{
653-
inti;
654647
StringInfoDatastr;
655648

649+
/* In TEXT mode, print nothing if there are no options */
650+
if (num <=0)
651+
return;
652+
656653
initStringInfo(&str);
657654

658-
for (i=0;i<num;i++)
655+
for (inti=0;i<num;i++)
659656
{
660657
char*setting;
661658
structconfig_generic*conf=gucs[i];
@@ -671,8 +668,7 @@ ExplainPrintSettings(ExplainState *es)
671668
appendStringInfo(&str,"%s = NULL",conf->name);
672669
}
673670

674-
if (num>0)
675-
ExplainPropertyText("Settings",str.data,es);
671+
ExplainPropertyText("Settings",str.data,es);
676672
}
677673
}
678674

‎src/backend/utils/misc/guc.c

Lines changed: 15 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4663,9 +4663,6 @@ static struct config_generic **guc_variables;
46634663
/* Current number of variables contained in the vector */
46644664
staticintnum_guc_variables;
46654665

4666-
/* Current number of variables marked with GUC_EXPLAIN */
4667-
staticintnum_guc_explain_variables;
4668-
46694666
/* Vector capacity */
46704667
staticintsize_guc_variables;
46714668

@@ -4929,7 +4926,6 @@ build_guc_variables(void)
49294926
{
49304927
intsize_vars;
49314928
intnum_vars=0;
4932-
intnum_explain_vars=0;
49334929
structconfig_generic**guc_vars;
49344930
inti;
49354931

@@ -4940,9 +4936,6 @@ build_guc_variables(void)
49404936
/* Rather than requiring vartype to be filled in by hand, do this: */
49414937
conf->gen.vartype=PGC_BOOL;
49424938
num_vars++;
4943-
4944-
if (conf->gen.flags&GUC_EXPLAIN)
4945-
num_explain_vars++;
49464939
}
49474940

49484941
for (i=0;ConfigureNamesInt[i].gen.name;i++)
@@ -4951,9 +4944,6 @@ build_guc_variables(void)
49514944

49524945
conf->gen.vartype=PGC_INT;
49534946
num_vars++;
4954-
4955-
if (conf->gen.flags&GUC_EXPLAIN)
4956-
num_explain_vars++;
49574947
}
49584948

49594949
for (i=0;ConfigureNamesReal[i].gen.name;i++)
@@ -4962,9 +4952,6 @@ build_guc_variables(void)
49624952

49634953
conf->gen.vartype=PGC_REAL;
49644954
num_vars++;
4965-
4966-
if (conf->gen.flags&GUC_EXPLAIN)
4967-
num_explain_vars++;
49684955
}
49694956

49704957
for (i=0;ConfigureNamesString[i].gen.name;i++)
@@ -4973,9 +4960,6 @@ build_guc_variables(void)
49734960

49744961
conf->gen.vartype=PGC_STRING;
49754962
num_vars++;
4976-
4977-
if (conf->gen.flags&GUC_EXPLAIN)
4978-
num_explain_vars++;
49794963
}
49804964

49814965
for (i=0;ConfigureNamesEnum[i].gen.name;i++)
@@ -4984,9 +4968,6 @@ build_guc_variables(void)
49844968

49854969
conf->gen.vartype=PGC_ENUM;
49864970
num_vars++;
4987-
4988-
if (conf->gen.flags&GUC_EXPLAIN)
4989-
num_explain_vars++;
49904971
}
49914972

49924973
/*
@@ -5018,7 +4999,6 @@ build_guc_variables(void)
50184999
free(guc_variables);
50195000
guc_variables=guc_vars;
50205001
num_guc_variables=num_vars;
5021-
num_guc_explain_variables=num_explain_vars;
50225002
size_guc_variables=size_vars;
50235003
qsort((void*)guc_variables,num_guc_variables,
50245004
sizeof(structconfig_generic*),guc_var_compare);
@@ -8967,41 +8947,40 @@ ShowAllGUCConfig(DestReceiver *dest)
89678947
}
89688948

89698949
/*
8970-
* Returns an array of modified GUC options to show in EXPLAIN. Only options
8971-
* related to query planning (marked with GUC_EXPLAIN), with values different
8972-
* from built-in defaults.
8950+
* Return an array of modified GUC options to show in EXPLAIN.
8951+
*
8952+
* We only report options related to query planning (marked with GUC_EXPLAIN),
8953+
* with values different from their built-in defaults.
89738954
*/
89748955
structconfig_generic**
89758956
get_explain_guc_options(int*num)
89768957
{
8977-
inti;
89788958
structconfig_generic**result;
89798959

89808960
*num=0;
89818961

89828962
/*
8983-
* Allocate enough space to fit all GUC_EXPLAIN options. We may not need
8984-
* all the space, but there are fairly few such options so we don't waste
8985-
* a lot of memory.
8963+
* While only a fraction of all the GUC variables are marked GUC_EXPLAIN,
8964+
* it doesn't seem worth dynamically resizing this array.
89868965
*/
8987-
result=palloc(sizeof(structconfig_generic*)*num_guc_explain_variables);
8966+
result=palloc(sizeof(structconfig_generic*)*num_guc_variables);
89888967

8989-
for (i=0;i<num_guc_variables;i++)
8968+
for (inti=0;i<num_guc_variables;i++)
89908969
{
89918970
boolmodified;
89928971
structconfig_generic*conf=guc_variables[i];
89938972

8994-
/* return only options visible to the user */
8973+
/* return only parameters marked for inclusion in explain */
8974+
if (!(conf->flags&GUC_EXPLAIN))
8975+
continue;
8976+
8977+
/* return only options visible to the current user */
89958978
if ((conf->flags&GUC_NO_SHOW_ALL)||
89968979
((conf->flags&GUC_SUPERUSER_ONLY)&&
89978980
!is_member_of_role(GetUserId(),DEFAULT_ROLE_READ_ALL_SETTINGS)))
89988981
continue;
89998982

9000-
/* only parameters explicitly marked for inclusion in explain */
9001-
if (!(conf->flags&GUC_EXPLAIN))
9002-
continue;
9003-
9004-
/* return only options that were modified (w.r.t. config file) */
8983+
/* return only options that are different from their boot values */
90058984
modified= false;
90068985

90078986
switch (conf->vartype)
@@ -9050,15 +9029,12 @@ get_explain_guc_options(int *num)
90509029
elog(ERROR,"unexpected GUC type: %d",conf->vartype);
90519030
}
90529031

9053-
/* skip GUC variables that match the built-in default */
90549032
if (!modified)
90559033
continue;
90569034

9057-
/*assign to the values array */
9035+
/*OK, report it */
90589036
result[*num]=conf;
90599037
*num=*num+1;
9060-
9061-
Assert(*num <=num_guc_explain_variables);
90629038
}
90639039

90649040
returnresult;

‎src/test/regress/expected/explain.out

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,26 @@ select explain_filter('explain (analyze, buffers, format yaml) select * from int
181181
Execution Time: N.N
182182
(1 row)
183183

184+
-- SETTINGS option
185+
-- We have to ignore other settings that might be imposed by the environment,
186+
-- so printing the whole Settings field unfortunately won't do.
187+
begin;
188+
set local plan_cache_mode = force_generic_plan;
189+
select true as "OK"
190+
from explain_filter('explain (settings) select * from int8_tbl i8') ln
191+
where ln ~ '^ *Settings: .*plan_cache_mode = ''force_generic_plan''';
192+
OK
193+
----
194+
t
195+
(1 row)
196+
197+
select explain_filter_to_json('explain (settings, format json) select * from int8_tbl i8') #> '{0,Settings,plan_cache_mode}';
198+
?column?
199+
----------------------
200+
"force_generic_plan"
201+
(1 row)
202+
203+
rollback;
184204
--
185205
-- Test production of per-worker data
186206
--

‎src/test/regress/sql/explain.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,18 @@ select explain_filter('explain (analyze, buffers, format json) select * from int
5757
select explain_filter('explain (analyze, buffers, format xml) select * from int8_tbl i8');
5858
select explain_filter('explain (analyze, buffers, format yaml) select * from int8_tbl i8');
5959

60+
-- SETTINGS option
61+
-- We have to ignore other settings that might be imposed by the environment,
62+
-- so printing the whole Settings field unfortunately won't do.
63+
64+
begin;
65+
set local plan_cache_mode= force_generic_plan;
66+
select trueas"OK"
67+
from explain_filter('explain (settings) select * from int8_tbl i8') ln
68+
where ln ~'^ *Settings: .*plan_cache_mode =''force_generic_plan''';
69+
select explain_filter_to_json('explain (settings, format json) select * from int8_tbl i8')#> '{0,Settings,plan_cache_mode}';
70+
rollback;
71+
6072
--
6173
-- Test production of per-worker data
6274
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp