public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 9/9] Ignore correlation for new BRIN opclasses 3+ messages / 3 participants [nested] [flat]
* [PATCH 9/9] Ignore correlation for new BRIN opclasses @ 2020-09-12 13:07 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw) The new BRIN opclasses (bloom and minmax-multi) are less sensitive to poorly correlated data, so just assume the data is perfectly correlated during costing. Author: Tomas Vondra <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/brin/brin_bloom.c | 1 + src/backend/access/brin/brin_minmax_multi.c | 1 + src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++- src/include/access/brin_internal.h | 3 +++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c index b54b963f87..af87737ae0 100644 --- a/src/backend/access/brin/brin_bloom.c +++ b/src/backend/access/brin/brin_bloom.c @@ -410,6 +410,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(BloomOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (BloomOpaque *) diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index d11ec73c81..4152b38c9e 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -1798,6 +1798,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(MinmaxMultiOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (MinmaxMultiOpaque *) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 47ca4ddbb5..bf40f9ce32 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -98,6 +98,7 @@ #include <math.h> #include "access/brin.h" +#include "access/brin_internal.h" #include "access/brin_page.h" #include "access/gin.h" #include "access/table.h" @@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, double minimalRanges; double estimatedRanges; double selec; - Relation indexRel; + Relation indexRel = NULL; + TupleDesc tupdesc = NULL; ListCell *l; VariableStatData vardata; @@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, */ indexRel = index_open(index->indexoid, NoLock); brinGetStats(indexRel, &statsData); + tupdesc = RelationGetDescr(indexRel); index_close(indexRel, NoLock); /* work out the actual number of ranges in the index */ @@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, { IndexClause *iclause = lfirst_node(IndexClause, l); AttrNumber attnum = index->indexkeys[iclause->indexcol]; + FmgrInfo *opcInfoFn; + BrinOpcInfo *opcInfo; + Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol); + bool ignore_correlation; + + opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO); + + opcInfo = (BrinOpcInfo *) + DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid)); + + ignore_correlation = opcInfo->oi_ignore_correlation; /* attempt to lookup stats in relation for this index column */ if (attnum != 0) @@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, if (sslot.nnumbers > 0) varCorrelation = Abs(sslot.numbers[0]); + if (ignore_correlation) + varCorrelation = 1.0; + if (varCorrelation > *indexCorrelation) *indexCorrelation = varCorrelation; diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index fdaff42722..5fbf8cf9c7 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -30,6 +30,9 @@ typedef struct BrinOpcInfo /* Regular processing of NULLs in BrinValues? */ bool oi_regular_nulls; + /* Ignore correlation during cost estimation */ + bool oi_ignore_correlation; + /* Opaque pointer for the opclass' private use */ void *oi_opaque; -- 2.26.2 --------------0E72B707603BED22B4040825 Content-Type: text/x-patch; charset=UTF-8; name="0008-Define-multi-minmax-oclasses-for-types-with-20210211.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0008-Define-multi-minmax-oclasses-for-types-with-20210211.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: GUC flags @ 2022-01-05 23:55 Justin Pryzby <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Justin Pryzby @ 2022-01-05 23:55 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected] On Wed, Jan 05, 2022 at 02:17:11PM +0900, Michael Paquier wrote: > On Tue, Jan 04, 2022 at 09:06:48PM -0600, Justin Pryzby wrote: > > I think pg_get_guc_flags() may be best, but I'm interested to hear other > > opinions. > > My opinion on this matter is rather close to what you have here with > handling things through one extra attribute. But I don't see the > point of using an extra function where users would need to do a manual > mapping of the flag bits back to a a text representation of them. If it were implemented as a function, this would be essentially internal and left undocumented. Only exposed for the purpose of re-implementing check_guc. > I would suggest to just add one text[] to pg_show_all_settings Good idea to use the backing function without updating the view. pg_settings is currently defined with "SELECT *". Is it fine to enumerate a list of columns instead ? Attachments: [text/x-diff] v4-0001-check_guc-fix-absurd-number-of-false-positives.patch (3.9K, ../../[email protected]/2-v4-0001-check_guc-fix-absurd-number-of-false-positives.patch) download | inline diff: From 713046d82668c4e189b78e9e274b272bccaaa480 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <[email protected]> Date: Sun, 5 Dec 2021 23:22:04 -0600 Subject: [PATCH v4 1/2] check_guc: fix absurd number of false positives Avoid false positives; Avoid various assumptions; Avoid list of exceptions; Simplify shell/awk/sed/grep; This requires GNU awk for RS as a regex. --- src/backend/utils/misc/check_guc | 69 +++++--------------------------- 1 file changed, 10 insertions(+), 59 deletions(-) diff --git a/src/backend/utils/misc/check_guc b/src/backend/utils/misc/check_guc index b171ef0e4ff..323ca13191b 100755 --- a/src/backend/utils/misc/check_guc +++ b/src/backend/utils/misc/check_guc @@ -1,78 +1,29 @@ -#!/bin/sh +#! /bin/sh +set -e -## currently, this script makes a lot of assumptions: +## this script makes some assumptions about the formatting of files it parses. ## in postgresql.conf.sample: ## 1) the valid config settings may be preceded by a '#', but NOT '# ' ## (we use this to skip comments) -## 2) the valid config settings will be followed immediately by ' =' -## (at least one space preceding the '=') -## in guc.c: -## 3) the options have PGC_ on the same line as the option -## 4) the options have '{' on the same line as the option - -## Problems -## 1) Don't know what to do with TRANSACTION ISOLATION LEVEL - -## if an option is valid but shows up in only one file (guc.c but not -## postgresql.conf.sample), it should be listed here so that it -## can be ignored -INTENTIONALLY_NOT_INCLUDED="debug_deadlocks in_hot_standby \ -is_superuser lc_collate lc_ctype lc_messages lc_monetary lc_numeric lc_time \ -pre_auth_delay role seed server_encoding server_version server_version_num \ -session_authorization trace_lock_oidmin trace_lock_table trace_locks trace_lwlocks \ -trace_notify trace_userlocks transaction_isolation transaction_read_only \ -zero_damaged_pages" ### What options are listed in postgresql.conf.sample, but don't appear ### in guc.c? -# grab everything that looks like a setting and convert it to lower case -SETTINGS=`grep ' =' postgresql.conf.sample | -grep -v '^# ' | # strip comments -sed -e 's/^#//' | -awk '{print $1}'` - -SETTINGS=`echo "$SETTINGS" | tr 'A-Z' 'a-z'` +# grab everything that looks like a setting +SETTINGS=`sed '/^#[[:alnum:]]/!d; s/^#//; s/ =.*//; /^include/d' postgresql.conf.sample` for i in $SETTINGS ; do - hidden=0 ## it sure would be nice to replace this with an sql "not in" statement - ## it doesn't seem to make sense to have things in .sample and not in guc.c -# for hidethis in $INTENTIONALLY_NOT_INCLUDED ; do -# if [ "$hidethis" = "$i" ] ; then -# hidden=1 -# fi -# done - if [ "$hidden" -eq 0 ] ; then - grep -i '"'$i'"' guc.c > /dev/null - if [ $? -ne 0 ] ; then - echo "$i seems to be missing from guc.c"; - fi; - fi + grep -i "\"$i\"" guc.c >/dev/null || + echo "$i seems to be missing from guc.c"; done ### What options are listed in guc.c, but don't appear ### in postgresql.conf.sample? # grab everything that looks like a setting and convert it to lower case - -SETTINGS=`grep '{.* PGC_' guc.c | awk '{print $1}' | \ - sed -e 's/{//g' -e 's/"//g' -e 's/,//'` - -SETTINGS=`echo "$SETTINGS" | tr 'A-Z' 'a-z'` - +SETTINGS=`gawk -F '[",]' 'BEGIN{RS="\n\t\\\\{\n"} /",[[:space:]]*PGC_.*.*gettext_noop/ && !/NOT_IN_SAMPLE/{print tolower($2)}' guc.c` for i in $SETTINGS ; do - hidden=0 - ## it sure would be nice to replace this with an sql "not in" statement - for hidethis in $INTENTIONALLY_NOT_INCLUDED ; do - if [ "$hidethis" = "$i" ] ; then - hidden=1 - fi - done - if [ "$hidden" -eq 0 ] ; then - grep -i '#'$i' ' postgresql.conf.sample > /dev/null - if [ $? -ne 0 ] ; then - echo "$i seems to be missing from postgresql.conf.sample"; - fi - fi + grep "#$i " postgresql.conf.sample >/dev/null || + echo "$i seems to be missing from postgresql.conf.sample"; done -- 2.17.1 [text/x-diff] v4-0002-Expose-GUC-flags-in-SQL-function-retire-.-check_g.patch (15.9K, ../../[email protected]/3-v4-0002-Expose-GUC-flags-in-SQL-function-retire-.-check_g.patch) download | inline diff: From d4773ee874ddc6aaa1238e4d1ae7ec6f8e20f36e Mon Sep 17 00:00:00 2001 From: Justin Pryzby <[email protected]> Date: Wed, 8 Dec 2021 12:06:35 -0600 Subject: [PATCH v4 2/2] Expose GUC flags in SQL function; retire ./check_guc --- src/backend/catalog/system_views.sql | 19 ++++- src/backend/utils/misc/check_guc | 29 ------- src/backend/utils/misc/guc.c | 37 +++++++- src/include/catalog/pg_proc.dat | 6 +- src/test/regress/expected/guc.out | 122 +++++++++++++++++++++++++++ src/test/regress/expected/rules.out | 2 +- src/test/regress/sql/guc.sql | 72 ++++++++++++++++ 7 files changed, 252 insertions(+), 35 deletions(-) delete mode 100755 src/backend/utils/misc/check_guc diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 61b515cdb85..e2c01443cb6 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -580,7 +580,24 @@ FROM JOIN pg_authid rol ON l.classoid = rol.tableoid AND l.objoid = rol.oid; CREATE VIEW pg_settings AS - SELECT * FROM pg_show_all_settings() AS A; + SELECT a.name, + a.setting, + a.unit, + a.category, + a.short_desc, + a.extra_desc, + a.context, + a.vartype, + a.source, + a.min_val, + a.max_val, + a.enumvals, + a.boot_val, + a.reset_val, + a.sourcefile, + a.sourceline, + a.pending_restart + FROM pg_show_all_settings() AS A; CREATE RULE pg_settings_u AS ON UPDATE TO pg_settings diff --git a/src/backend/utils/misc/check_guc b/src/backend/utils/misc/check_guc deleted file mode 100755 index 323ca13191b..00000000000 --- a/src/backend/utils/misc/check_guc +++ /dev/null @@ -1,29 +0,0 @@ -#! /bin/sh -set -e - -## this script makes some assumptions about the formatting of files it parses. -## in postgresql.conf.sample: -## 1) the valid config settings may be preceded by a '#', but NOT '# ' -## (we use this to skip comments) - -### What options are listed in postgresql.conf.sample, but don't appear -### in guc.c? - -# grab everything that looks like a setting -SETTINGS=`sed '/^#[[:alnum:]]/!d; s/^#//; s/ =.*//; /^include/d' postgresql.conf.sample` - -for i in $SETTINGS ; do - ## it sure would be nice to replace this with an sql "not in" statement - grep -i "\"$i\"" guc.c >/dev/null || - echo "$i seems to be missing from guc.c"; -done - -### What options are listed in guc.c, but don't appear -### in postgresql.conf.sample? - -# grab everything that looks like a setting and convert it to lower case -SETTINGS=`gawk -F '[",]' 'BEGIN{RS="\n\t\\\\{\n"} /",[[:space:]]*PGC_.*.*gettext_noop/ && !/NOT_IN_SAMPLE/{print tolower($2)}' guc.c` -for i in $SETTINGS ; do - grep "#$i " postgresql.conf.sample >/dev/null || - echo "$i seems to be missing from postgresql.conf.sample"; -done diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index f9504d3aec4..9b6ea9279bd 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -9622,6 +9622,36 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok) return _ShowOption(record, true); } +/* + * Return an text[] array of the given flags (or a useful subset?) XXX + */ +static char * +get_flags_array_text(int flags) +{ + StringInfoData ret; + + initStringInfo(&ret); + appendStringInfoChar(&ret, '{'); + + if (flags & GUC_NO_SHOW_ALL) + appendStringInfo(&ret, "NO_SHOW_ALL,"); + if (flags & GUC_NO_RESET_ALL) + appendStringInfo(&ret, "NO_RESET_ALL,"); + if (flags & GUC_NOT_IN_SAMPLE) + appendStringInfo(&ret, "NOT_IN_SAMPLE,"); + if (flags & GUC_EXPLAIN) + appendStringInfo(&ret, "EXPLAIN,"); + if (flags & GUC_RUNTIME_COMPUTED) + appendStringInfo(&ret, "RUNTIME_COMPUTED,"); + + /* Remove trailing comma, if any */ + if (ret.len > 1) + ret.data[--ret.len] = '\0'; + + appendStringInfoChar(&ret, '}'); + return ret.data; +} + /* * Return GUC variable value by variable number; optionally return canonical * form of name. Return value is palloc'd. @@ -9849,6 +9879,9 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow) } values[16] = (conf->status & GUC_PENDING_RESTART) ? "t" : "f"; + + /* flags */ + values[17] = get_flags_array_text(conf->flags); } /* @@ -9904,7 +9937,7 @@ show_config_by_name_missing_ok(PG_FUNCTION_ARGS) * show_all_settings - equiv to SHOW ALL command but implemented as * a Table Function. */ -#define NUM_PG_SETTINGS_ATTS 17 +#define NUM_PG_SETTINGS_ATTS 18 Datum show_all_settings(PG_FUNCTION_ARGS) @@ -9966,6 +9999,8 @@ show_all_settings(PG_FUNCTION_ARGS) INT4OID, -1, 0); TupleDescInitEntry(tupdesc, (AttrNumber) 17, "pending_restart", BOOLOID, -1, 0); + TupleDescInitEntry(tupdesc, (AttrNumber) 18, "flags", + TEXTARRAYOID, -1, 0); /* * Generate attribute metadata needed later to produce tuples from raw diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 4d992dc2241..7e7e9a48fac 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6092,9 +6092,9 @@ { oid => '2084', descr => 'SHOW ALL as a function', proname => 'pg_show_all_settings', prorows => '1000', proretset => 't', provolatile => 's', prorettype => 'record', proargtypes => '', - proallargtypes => '{text,text,text,text,text,text,text,text,text,text,text,_text,text,text,text,int4,bool}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,boot_val,reset_val,sourcefile,sourceline,pending_restart}', + proallargtypes => '{text,text,text,text,text,text,text,text,text,text,text,_text,text,text,text,int4,bool,_text}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,boot_val,reset_val,sourcefile,sourceline,pending_restart,flags}', prosrc => 'show_all_settings' }, { oid => '3329', descr => 'show config file settings', proname => 'pg_show_all_file_settings', prorows => '1000', proretset => 't', diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 59da91ff04d..d27b7c24747 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -813,3 +813,125 @@ set default_with_oids to f; -- Should not allow to set it to true. set default_with_oids to t; ERROR: tables declared WITH OIDS are not supported +-- +-- Test GUC categories and flags. +-- +CREATE TABLE pg_settings_flags AS SELECT name, category, + 'NO_SHOW_ALL' =ANY(flags) AS no_show_all, + 'NO_RESET_ALL' =ANY(flags) AS no_reset_all, + 'NOT_IN_SAMPLE' =ANY(flags) AS not_in_sample, + 'EXPLAIN' =ANY(flags) AS guc_explain, + 'COMPUTED' =ANY(flags) AS guc_computed + FROM pg_show_all_settings(); +-- test that GUCS are in postgresql.conf +SELECT lower(name) FROM pg_settings_flags WHERE NOT not_in_sample EXCEPT +SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) = .*', '\1') AS guc +FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf +WHERE ln ~ '^#?[[:alpha:]]' +ORDER BY 1; + lower +----------------------------- + config_file + plpgsql.check_asserts + plpgsql.extra_errors + plpgsql.extra_warnings + plpgsql.print_strict_params + plpgsql.variable_conflict +(6 rows) + +-- test that lines in postgresql.conf that look like GUCs are GUCs +SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) = .*', '\1') AS guc +FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf +WHERE ln ~ '^#?[[:alpha:]]' +EXCEPT SELECT lower(name) FROM pg_settings_flags WHERE NOT not_in_sample +ORDER BY 1; + guc +------------------- + include + include_dir + include_if_exists +(3 rows) + +-- test that section:DEVELOPER GUCs are flagged GUC_NOT_IN_SAMPLE: +SELECT * FROM pg_settings_flags +WHERE category='Developer Options' AND NOT not_in_sample +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------+----------+-------------+--------------+---------------+-------------+-------------- +(0 rows) + +-- Maybe the converse: +SELECT * FROM pg_settings_flags +WHERE category NOT IN ('Developer Options', 'Preset Options') AND not_in_sample +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------------------------+-------------------------------------------------+-------------+--------------+---------------+-------------+-------------- + application_name | Reporting and Logging / What to Log | f | f | t | f | f + transaction_deferrable | Client Connection Defaults / Statement Behavior | f | t | t | f | f + transaction_isolation | Client Connection Defaults / Statement Behavior | f | t | t | f | f + transaction_read_only | Client Connection Defaults / Statement Behavior | f | t | t | f | f +(4 rows) + +-- Most Query Tuning GUCs are flagged as EXPLAIN: +SELECT * FROM pg_settings_flags +WHERE category ~ '^Query Tuning' AND NOT guc_explain +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +---------------------------+--------------------------------------+-------------+--------------+---------------+-------------+-------------- + default_statistics_target | Query Tuning / Other Planner Options | f | f | f | f | f +(1 row) + +-- Maybe the converse: +SELECT * FROM pg_settings_flags +WHERE guc_explain AND NOT category ~ '^Query Tuning|^Resource Usage' +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +---------------------+-------------------------------------------------+-------------+--------------+---------------+-------------+-------------- + force_parallel_mode | Developer Options | f | f | t | t | f + search_path | Client Connection Defaults / Statement Behavior | f | f | f | t | f +(2 rows) + +-- GUCs flagged RUNTIME are Preset +SELECT * FROM pg_settings_flags +WHERE guc_computed AND NOT category='Preset Options' +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------+----------+-------------+--------------+---------------+-------------+-------------- +(0 rows) + +-- PRESET GUCs are flagged NOT_IN_SAMPLE +SELECT * FROM pg_settings_flags +WHERE category='Preset Options' AND NOT not_in_sample +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------+----------+-------------+--------------+---------------+-------------+-------------- +(0 rows) + +-- NO_SHOW_ALL implies NO_RESET_ALL: +SELECT * FROM pg_settings_flags +WHERE no_show_all AND NOT no_reset_all +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------+----------+-------------+--------------+---------------+-------------+-------------- +(0 rows) + +-- Usually the converse: +SELECT * FROM pg_settings_flags +WHERE NOT no_show_all AND no_reset_all +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------------------------+-------------------------------------------------+-------------+--------------+---------------+-------------+-------------- + transaction_deferrable | Client Connection Defaults / Statement Behavior | f | t | t | f | f + transaction_isolation | Client Connection Defaults / Statement Behavior | f | t | t | f | f + transaction_read_only | Client Connection Defaults / Statement Behavior | f | t | t | f | f +(3 rows) + +-- NO_SHOW_ALL implies NOT_IN_SAMPLE: +SELECT * FROM pg_settings_flags +WHERE no_show_all AND NOT not_in_sample +ORDER BY 1; + name | category | no_show_all | no_reset_all | not_in_sample | guc_explain | guc_computed +------+----------+-------------+--------------+---------------+-------------+-------------- +(0 rows) + +DROP TABLE pg_settings_flags; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index b58b062b10d..71cbdc37233 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1725,7 +1725,7 @@ pg_settings| SELECT a.name, a.sourcefile, a.sourceline, a.pending_restart - FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, sourcefile, sourceline, pending_restart); + FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, sourcefile, sourceline, pending_restart, flags); pg_shadow| SELECT pg_authid.rolname AS usename, pg_authid.oid AS usesysid, pg_authid.rolcreatedb AS usecreatedb, diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index c39c11388d5..72b4c367941 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -311,3 +311,75 @@ reset check_function_bodies; set default_with_oids to f; -- Should not allow to set it to true. set default_with_oids to t; + +-- +-- Test GUC categories and flags. +-- +CREATE TABLE pg_settings_flags AS SELECT name, category, + 'NO_SHOW_ALL' =ANY(flags) AS no_show_all, + 'NO_RESET_ALL' =ANY(flags) AS no_reset_all, + 'NOT_IN_SAMPLE' =ANY(flags) AS not_in_sample, + 'EXPLAIN' =ANY(flags) AS guc_explain, + 'COMPUTED' =ANY(flags) AS guc_computed + FROM pg_show_all_settings(); + +-- test that GUCS are in postgresql.conf +SELECT lower(name) FROM pg_settings_flags WHERE NOT not_in_sample EXCEPT +SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) = .*', '\1') AS guc +FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf +WHERE ln ~ '^#?[[:alpha:]]' +ORDER BY 1; + +-- test that lines in postgresql.conf that look like GUCs are GUCs +SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) = .*', '\1') AS guc +FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf +WHERE ln ~ '^#?[[:alpha:]]' +EXCEPT SELECT lower(name) FROM pg_settings_flags WHERE NOT not_in_sample +ORDER BY 1; + +-- test that section:DEVELOPER GUCs are flagged GUC_NOT_IN_SAMPLE: +SELECT * FROM pg_settings_flags +WHERE category='Developer Options' AND NOT not_in_sample +ORDER BY 1; + +-- Maybe the converse: +SELECT * FROM pg_settings_flags +WHERE category NOT IN ('Developer Options', 'Preset Options') AND not_in_sample +ORDER BY 1; + +-- Most Query Tuning GUCs are flagged as EXPLAIN: +SELECT * FROM pg_settings_flags +WHERE category ~ '^Query Tuning' AND NOT guc_explain +ORDER BY 1; + +-- Maybe the converse: +SELECT * FROM pg_settings_flags +WHERE guc_explain AND NOT category ~ '^Query Tuning|^Resource Usage' +ORDER BY 1; + +-- GUCs flagged RUNTIME are Preset +SELECT * FROM pg_settings_flags +WHERE guc_computed AND NOT category='Preset Options' +ORDER BY 1; + +-- PRESET GUCs are flagged NOT_IN_SAMPLE +SELECT * FROM pg_settings_flags +WHERE category='Preset Options' AND NOT not_in_sample +ORDER BY 1; + +-- NO_SHOW_ALL implies NO_RESET_ALL: +SELECT * FROM pg_settings_flags +WHERE no_show_all AND NOT no_reset_all +ORDER BY 1; + +-- Usually the converse: +SELECT * FROM pg_settings_flags +WHERE NOT no_show_all AND no_reset_all +ORDER BY 1; + +-- NO_SHOW_ALL implies NOT_IN_SAMPLE: +SELECT * FROM pg_settings_flags +WHERE no_show_all AND NOT not_in_sample +ORDER BY 1; + +DROP TABLE pg_settings_flags; -- 2.17.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: GUC flags @ 2022-01-06 05:19 Michael Paquier <[email protected]> parent: Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Michael Paquier @ 2022-01-06 05:19 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected] On Wed, Jan 05, 2022 at 05:55:17PM -0600, Justin Pryzby wrote: > pg_settings is currently defined with "SELECT *". Is it fine to enumerate a > list of columns instead ? I'd like to think that this is a better practice when it comes documenting the columns, but I don't see an actual need for this extra complication here. > + initStringInfo(&ret); > + appendStringInfoChar(&ret, '{'); > + > + if (flags & GUC_NO_SHOW_ALL) > + appendStringInfo(&ret, "NO_SHOW_ALL,"); > + if (flags & GUC_NO_RESET_ALL) > + appendStringInfo(&ret, "NO_RESET_ALL,"); > + if (flags & GUC_NOT_IN_SAMPLE) > + appendStringInfo(&ret, "NOT_IN_SAMPLE,"); > + if (flags & GUC_EXPLAIN) > + appendStringInfo(&ret, "EXPLAIN,"); > + if (flags & GUC_RUNTIME_COMPUTED) > + appendStringInfo(&ret, "RUNTIME_COMPUTED,"); > + > + /* Remove trailing comma, if any */ > + if (ret.len > 1) > + ret.data[--ret.len] = '\0'; The way of building the text array is incorrect here. See heap_tuple_infomask_flags() in pageinspect as an example with all the HEAP_* flags. I think that you should allocate an array of Datums, use CStringGetTextDatum() to assign each array element, wrapping the whole with construct_array() to build the final value for the parameter tuple. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../YdZ7zCymonPV4%2F%[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-01-06 05:19 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-12 13:07 [PATCH 9/9] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]> 2022-01-05 23:55 Re: GUC flags Justin Pryzby <[email protected]> 2022-01-06 05:19 ` Re: GUC flags Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox