Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pwXpr-0004b6-D7 for pgsql-hackers@arkaria.postgresql.org; Wed, 10 May 2023 00:37:39 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1pwXpo-0002rp-9R for pgsql-hackers@arkaria.postgresql.org; Wed, 10 May 2023 00:37:36 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pwXpn-0002rf-Qv for pgsql-hackers@lists.postgresql.org; Wed, 10 May 2023 00:37:36 +0000 Received: from mail-il1-x136.google.com ([2607:f8b0:4864:20::136]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1pwXpj-001frx-GA for pgsql-hackers@postgresql.org; Wed, 10 May 2023 00:37:34 +0000 Received: by mail-il1-x136.google.com with SMTP id e9e14a558f8ab-33179bbb900so15542205ab.3 for ; Tue, 09 May 2023 17:37:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telsasoft-com.20221208.gappssmtp.com; s=20221208; t=1683679049; x=1686271049; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=KY7BINrDlpdLOtvPFHKuzpocmCIbmhOjq+BIFW1GnaY=; b=UwodvB3qTO0Sh2HDPrDY1ruy3TuWpHQLKR5lW26Mvxxf0q/61EPJq+SJFdsAk/9G5O 2NthjqT5rcfA1oaYxQpXOgWJ/XDwxTPAa/ECn8xpnjYPZKbNuwCweuG1RvW7Zi1kEtht 6n11LfHgFPO4svqdkbXDnge7p9A+W7+SXGhTpDhSS4oKwhwx6Qr5T+Ug+iun+79qZQlO givKJvwJvRpI0e5pf0aOAUNwF/OrMxR1lWaCZXDUwVgxbTXL7CX3Wee1zTNLV3eXmAFT JD2lExy2AB6KYsN/nNPO24g96KFDtncwzC/pJHZPobAB8ysJH1dYHaCcIy5NKUqPRbPv NUtQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683679049; x=1686271049; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=KY7BINrDlpdLOtvPFHKuzpocmCIbmhOjq+BIFW1GnaY=; b=HzDmgdtHViiThyr2vweA23t4N2AgV7lK+aScymv5zpufTvlSgsBstNpd2hPczLiTJB +tgwKwB+0VzqivwpIrYuNa6C7aWbCmx17oEfvMKZUE4iHi0zTAxSl4zjiMK2R7P0uLhW i7AwRZ5E0DuXg/ccCC+dNnpU77RVr67NyCmxy3nxakpfbuymbvOddlNHZWNtsq9omgyo 3hlmFxLqwsLe1o2EEgMJM7z+gsq8cMh4yXYGgBhFZH/vJr8QiKpH5o3HYoZzVDk6z2Ev ofAHcqg2gXBS1Itm6RpSpPYJsbJuF2WcfpVe9teIgBhGXe1efqIU8yuNLRpkuDgfx8yY EjlA== X-Gm-Message-State: AC+VfDzL9DqXkqM18AdjHHx6nQOmaKlSNWPUwoWX/HtzSQaAcA2+ADrq qFGbGMWFnEzkSogZmLvFllSctQ== X-Google-Smtp-Source: ACHHUZ5qkrWKIyAy50Mhifi5BfBFSlcf3GXVQlXuGd46EGQwOpSfJSA0dq0ac3UCCOHVsI25knadMA== X-Received: by 2002:a92:d381:0:b0:32a:fd29:26a8 with SMTP id o1-20020a92d381000000b0032afd2926a8mr10120164ilo.13.1683679049607; Tue, 09 May 2023 17:37:29 -0700 (PDT) Received: from pryzbyj.telsasoft (charmander.telsasoft.com. [50.244.222.1]) by smtp.gmail.com with ESMTPSA id s10-20020a02cc8a000000b0041461f9d685sm3196047jap.16.2023.05.09.17.37.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 May 2023 17:37:28 -0700 (PDT) Received: by pryzbyj.telsasoft (Postfix, from userid 1000) id C4E4B801312; Tue, 9 May 2023 19:37:27 -0500 (CDT) Date: Tue, 9 May 2023 19:37:27 -0500 From: Justin Pryzby To: Andres Freund Cc: Michael Paquier , Kyotaro Horiguchi , nathandbossart@gmail.com, pgsql-hackers@postgresql.org Subject: Re: fix stats_fetch_consistency value in postgresql.conf.sample Message-ID: References: <20220616132306.GD29853@telsasoft.com> <20220617.094358.2201653243066915172.horikyota.ntt@gmail.com> <20220622230710.r2m4ex5tlf6mveyh@alap3.anarazel.de> <20220630.173801.662386227537019720.horikyota.ntt@gmail.com> <20220713173000.GD18011@telsasoft.com> <20220713234900.z4rniuaerkq34s4v@awork3.anarazel.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8KqcO45QTmavqrvZ" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --8KqcO45QTmavqrvZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Mar 29, 2023 at 11:03:59PM -0500, Justin Pryzby wrote: > On Wed, Jul 13, 2022 at 04:49:00PM -0700, Andres Freund wrote: > > On 2022-07-14 08:46:02 +0900, Michael Paquier wrote: > > > On Wed, Jul 13, 2022 at 12:30:00PM -0500, Justin Pryzby wrote: > > > > How did you make this list ? Was it by excluding things that failed for you ? > > > > > > > > cfbot is currently failing due to io_concurrency on windows. > > > > I think there are more GUC which should be included here. > > > > > > > > http://cfbot.cputube.org/kyotaro-horiguchi.html > > > > > > FWIW, I am not really a fan of making this test depend on a hardcoded > > > list of GUCs. > > > > I wonder if we should add flags indicating platform dependency etc to guc.c? > > That should allow to remove most of them? > > Michael commented on this, but on another thread, so I'm copying and > pasting it here. > > On Thu, Mar 23, 2023 at 08:59:57PM -0500, Justin Pryzby wrote: > > On Fri, Mar 24, 2023 at 10:24:43AM +0900, Michael Paquier wrote: > > > >> * Check consistency of GUC defaults between .sample.conf and pg_settings.boot_val > > > > - It looks like this was pretty active until last October and might > > > > have been ready to apply at least partially? But no further work or > > > > review has happened since. > > > > > > FWIW, I don't find much appealing the addition of two GUC flags for > > > only the sole purpose of that, > > > > The flags seem independently interesting - adding them here follows > > a suggestion Andres made in response to your complaint. > > 20220713234900.z4rniuaerkq34s4v@awork3.anarazel.de > > > > > particularly as we get a stronger > > > dependency between GUCs that can be switched dynamically at > > > initialization and at compile-time. > > > > What do you mean by "stronger dependency between GUCs" ? > > I'm still not clear what that means ? Michael ? This fixes an issue with the last version that failed with log_autovacuum_min_duration in cirrusci's pg_ci_base.conf. And now includes both a perl and a sql-based versions of the test - both of which rely on the flags. --8KqcO45QTmavqrvZ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-pg_settings_get_flags-add-DEFAULT_COMPILE-and-DEFAUL.patch" From 963b56636b9f7fd4a78e502c6acba07919518910 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 19 Jul 2022 08:31:56 -0500 Subject: [PATCH 1/2] pg_settings_get_flags(): add DEFAULT_COMPILE and DEFAULT_INITDB .. for settings which vary by ./configure or platform or initdb Note that this is independent from PGC_S_DYNAMIC_DEFAULT. --- doc/src/sgml/func.sgml | 15 ++++++ src/backend/utils/misc/guc_funcs.c | 6 ++- src/backend/utils/misc/guc_tables.c | 76 ++++++++++++++++++----------- src/include/utils/guc.h | 2 + 4 files changed, 69 insertions(+), 30 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index e13fce6f6b1..17069d2249e 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -24945,6 +24945,21 @@ SELECT collation for ('foo' COLLATE "de_DE"); FlagDescription + + + DEFAULT_COMPILE + Parameters with this flag have default values which vary by + platform, or compile-time options. + + + + + DEFAULT_INITDB + Parameters with this flag have default values which are + determined dynamically during initdb. + + + EXPLAIN Parameters with this flag are included in diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c index 52c361e9755..183943c1973 100644 --- a/src/backend/utils/misc/guc_funcs.c +++ b/src/backend/utils/misc/guc_funcs.c @@ -551,7 +551,7 @@ ShowAllGUCConfig(DestReceiver *dest) Datum pg_settings_get_flags(PG_FUNCTION_ARGS) { -#define MAX_GUC_FLAGS 6 +#define MAX_GUC_FLAGS 8 char *varname = TextDatumGetCString(PG_GETARG_DATUM(0)); struct config_generic *record; int cnt = 0; @@ -564,6 +564,10 @@ pg_settings_get_flags(PG_FUNCTION_ARGS) if (record == NULL) PG_RETURN_NULL(); + if (record->flags & GUC_DEFAULT_COMPILE) + flags[cnt++] = CStringGetTextDatum("DEFAULT_COMPILE"); + if (record->flags & GUC_DEFAULT_INITDB) + flags[cnt++] = CStringGetTextDatum("DEFAULT_INITDB"); if (record->flags & GUC_EXPLAIN) flags[cnt++] = CStringGetTextDatum("EXPLAIN"); if (record->flags & GUC_NO_RESET) diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 2f42cebaf62..94b0aa24a98 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1233,7 +1233,7 @@ struct config_bool ConfigureNamesBool[] = {"debug_assertions", PGC_INTERNAL, PRESET_OPTIONS, gettext_noop("Shows whether the running server has assertion checks enabled."), NULL, - GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_DEFAULT_COMPILE }, &assert_enabled, DEFAULT_ASSERT_ENABLED, @@ -1425,7 +1425,8 @@ struct config_bool ConfigureNamesBool[] = { {"update_process_title", PGC_SUSET, PROCESS_TITLE, gettext_noop("Updates the process title to show the active SQL command."), - gettext_noop("Enables updating of the process title every time a new SQL command is received by the server.") + gettext_noop("Enables updating of the process title every time a new SQL command is received by the server."), + GUC_DEFAULT_COMPILE }, &update_process_title, DEFAULT_UPDATE_PROCESS_TITLE, @@ -2180,7 +2181,8 @@ struct config_int ConfigureNamesInt[] = { {"max_connections", PGC_POSTMASTER, CONN_AUTH_SETTINGS, gettext_noop("Sets the maximum number of concurrent connections."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &MaxConnections, 100, 1, MAX_BACKENDS, @@ -2228,7 +2230,7 @@ struct config_int ConfigureNamesInt[] = {"shared_buffers", PGC_POSTMASTER, RESOURCES_MEM, gettext_noop("Sets the number of shared memory buffers used by the server."), NULL, - GUC_UNIT_BLOCKS + GUC_UNIT_BLOCKS | GUC_DEFAULT_INITDB }, &NBuffers, 16384, 16, INT_MAX / 2, @@ -2282,7 +2284,8 @@ struct config_int ConfigureNamesInt[] = { {"port", PGC_POSTMASTER, CONN_AUTH_SETTINGS, gettext_noop("Sets the TCP port the server listens on."), - NULL + NULL, + GUC_DEFAULT_COMPILE }, &PostPortNumber, DEF_PGPORT, 1, 65535, @@ -2311,7 +2314,8 @@ struct config_int ConfigureNamesInt[] = "to be a numeric mode specification in the form " "accepted by the chmod and umask system calls. " "(To use the customary octal format the number must " - "start with a 0 (zero).)") + "start with a 0 (zero).)"), + GUC_DEFAULT_INITDB }, &Log_file_mode, 0600, 0000, 0777, @@ -2744,7 +2748,7 @@ struct config_int ConfigureNamesInt[] = {"checkpoint_flush_after", PGC_SIGHUP, WAL_CHECKPOINTS, gettext_noop("Number of pages after which previously performed writes are flushed to disk."), NULL, - GUC_UNIT_BLOCKS + GUC_UNIT_BLOCKS | GUC_DEFAULT_COMPILE }, &checkpoint_flush_after, DEFAULT_CHECKPOINT_FLUSH_AFTER, 0, WRITEBACK_MAX_PENDING_FLUSHES, @@ -2962,7 +2966,7 @@ struct config_int ConfigureNamesInt[] = {"bgwriter_flush_after", PGC_SIGHUP, RESOURCES_BGWRITER, gettext_noop("Number of pages after which previously performed writes are flushed to disk."), NULL, - GUC_UNIT_BLOCKS + GUC_UNIT_BLOCKS | GUC_DEFAULT_COMPILE }, &bgwriter_flush_after, DEFAULT_BGWRITER_FLUSH_AFTER, 0, WRITEBACK_MAX_PENDING_FLUSHES, @@ -2975,7 +2979,7 @@ struct config_int ConfigureNamesInt[] = RESOURCES_ASYNCHRONOUS, gettext_noop("Number of simultaneous requests that can be handled efficiently by the disk subsystem."), NULL, - GUC_EXPLAIN + GUC_EXPLAIN | GUC_DEFAULT_COMPILE }, &effective_io_concurrency, DEFAULT_EFFECTIVE_IO_CONCURRENCY, @@ -2989,7 +2993,7 @@ struct config_int ConfigureNamesInt[] = RESOURCES_ASYNCHRONOUS, gettext_noop("A variant of effective_io_concurrency that is used for maintenance work."), NULL, - GUC_EXPLAIN + GUC_EXPLAIN | GUC_DEFAULT_COMPILE }, &maintenance_io_concurrency, DEFAULT_MAINTENANCE_IO_CONCURRENCY, @@ -3002,7 +3006,7 @@ struct config_int ConfigureNamesInt[] = {"backend_flush_after", PGC_USERSET, RESOURCES_ASYNCHRONOUS, gettext_noop("Number of pages after which previously performed writes are flushed to disk."), NULL, - GUC_UNIT_BLOCKS + GUC_UNIT_BLOCKS | GUC_DEFAULT_COMPILE }, &backend_flush_after, DEFAULT_BACKEND_FLUSH_AFTER, 0, WRITEBACK_MAX_PENDING_FLUSHES, @@ -3457,7 +3461,7 @@ struct config_int ConfigureNamesInt[] = {"debug_discard_caches", PGC_SUSET, DEVELOPER_OPTIONS, gettext_noop("Aggressively flush system caches for debugging purposes."), NULL, - GUC_NOT_IN_SAMPLE + GUC_NOT_IN_SAMPLE | GUC_DEFAULT_COMPILE }, &debug_discard_caches, #ifdef DISCARD_CACHES_ENABLED @@ -3951,7 +3955,8 @@ struct config_string ConfigureNamesString[] = { {"log_timezone", PGC_SIGHUP, LOGGING_WHAT, gettext_noop("Sets the time zone to use in log messages."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &log_timezone_string, "GMT", @@ -3963,7 +3968,7 @@ struct config_string ConfigureNamesString[] = gettext_noop("Sets the display format for date and time values."), gettext_noop("Also controls interpretation of ambiguous " "date inputs."), - GUC_LIST_INPUT | GUC_REPORT + GUC_LIST_INPUT | GUC_REPORT | GUC_DEFAULT_INITDB }, &datestyle_string, "ISO, MDY", @@ -4077,7 +4082,8 @@ struct config_string ConfigureNamesString[] = { {"lc_messages", PGC_SUSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the language in which messages are displayed."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &locale_messages, "", @@ -4087,7 +4093,8 @@ struct config_string ConfigureNamesString[] = { {"lc_monetary", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the locale for formatting monetary amounts."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &locale_monetary, "C", @@ -4097,7 +4104,8 @@ struct config_string ConfigureNamesString[] = { {"lc_numeric", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the locale for formatting numbers."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &locale_numeric, "C", @@ -4107,7 +4115,8 @@ struct config_string ConfigureNamesString[] = { {"lc_time", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the locale for formatting date and time values."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &locale_time, "C", @@ -4266,7 +4275,8 @@ struct config_string ConfigureNamesString[] = {"TimeZone", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the time zone for displaying and interpreting time stamps."), NULL, - GUC_REPORT + GUC_REPORT | GUC_DEFAULT_INITDB + }, &timezone_string, "GMT", @@ -4297,7 +4307,7 @@ struct config_string ConfigureNamesString[] = {"unix_socket_directories", PGC_POSTMASTER, CONN_AUTH_SETTINGS, gettext_noop("Sets the directories where Unix-domain sockets will be created."), NULL, - GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_SUPERUSER_ONLY + GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_SUPERUSER_ONLY | GUC_DEFAULT_COMPILE }, &Unix_socket_directories, DEFAULT_PGSOCKET_DIR, @@ -4378,7 +4388,7 @@ struct config_string ConfigureNamesString[] = {"ssl_library", PGC_INTERNAL, PRESET_OPTIONS, gettext_noop("Shows the name of the SSL library."), NULL, - GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_DEFAULT_COMPILE }, &ssl_library, #ifdef USE_SSL @@ -4453,7 +4463,9 @@ struct config_string ConfigureNamesString[] = { {"default_text_search_config", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets default text search configuration."), - NULL + NULL, + GUC_DEFAULT_INITDB + }, &TSCurrentConfig, "pg_catalog.simple", @@ -4464,7 +4476,7 @@ struct config_string ConfigureNamesString[] = {"ssl_ciphers", PGC_SIGHUP, CONN_AUTH_SSL, gettext_noop("Sets the list of allowed SSL ciphers."), NULL, - GUC_SUPERUSER_ONLY + GUC_SUPERUSER_ONLY | GUC_DEFAULT_COMPILE }, &SSLCipherSuites, #ifdef USE_OPENSSL @@ -4479,7 +4491,7 @@ struct config_string ConfigureNamesString[] = {"ssl_ecdh_curve", PGC_SIGHUP, CONN_AUTH_SSL, gettext_noop("Sets the curve to use for ECDH."), NULL, - GUC_SUPERUSER_ONLY + GUC_SUPERUSER_ONLY | GUC_DEFAULT_COMPILE }, &SSLECDHCurve, #ifdef USE_SSL @@ -4738,7 +4750,8 @@ struct config_enum ConfigureNamesEnum[] = { {"syslog_facility", PGC_SIGHUP, LOGGING_WHERE, gettext_noop("Sets the syslog \"facility\" to be used when syslog enabled."), - NULL + NULL, + GUC_DEFAULT_COMPILE }, &syslog_facility, DEFAULT_SYSLOG_FACILITY, @@ -4847,7 +4860,9 @@ struct config_enum ConfigureNamesEnum[] = { {"dynamic_shared_memory_type", PGC_POSTMASTER, RESOURCES_MEM, gettext_noop("Selects the dynamic shared memory implementation used."), - NULL + NULL, + /* platform-specific default, which is also overriden during initdb */ + GUC_DEFAULT_COMPILE | GUC_DEFAULT_INITDB }, &dynamic_shared_memory_type, DEFAULT_DYNAMIC_SHARED_MEMORY_TYPE, dynamic_shared_memory_options, @@ -4857,7 +4872,8 @@ struct config_enum ConfigureNamesEnum[] = { {"shared_memory_type", PGC_POSTMASTER, RESOURCES_MEM, gettext_noop("Selects the shared memory implementation used for the main shared memory region."), - NULL + NULL, + GUC_DEFAULT_COMPILE }, &shared_memory_type, DEFAULT_SHARED_MEMORY_TYPE, shared_memory_options, @@ -4867,7 +4883,8 @@ struct config_enum ConfigureNamesEnum[] = { {"wal_sync_method", PGC_SIGHUP, WAL_SETTINGS, gettext_noop("Selects the method used for forcing WAL updates to disk."), - NULL + NULL, + GUC_DEFAULT_COMPILE }, &sync_method, DEFAULT_SYNC_METHOD, sync_method_options, @@ -4931,7 +4948,8 @@ struct config_enum ConfigureNamesEnum[] = { {"password_encryption", PGC_USERSET, CONN_AUTH_AUTH, gettext_noop("Chooses the algorithm for encrypting passwords."), - NULL + NULL, + GUC_DEFAULT_INITDB }, &Password_encryption, PASSWORD_TYPE_SCRAM_SHA_256, password_encryption_options, diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index ba89d013e64..2b98595b3ea 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -221,6 +221,8 @@ typedef enum #define GUC_DISALLOW_IN_AUTO_FILE \ 0x002000 /* can't set in PG_AUTOCONF_FILENAME */ #define GUC_RUNTIME_COMPUTED 0x004000 /* delay processing in 'postgres -C' */ +#define GUC_DEFAULT_COMPILE 0x008000 /* default is determined at compile time */ +#define GUC_DEFAULT_INITDB 0x010000 /* default is determined at during initdb */ #define GUC_UNIT_KB 0x01000000 /* value is in kilobytes */ #define GUC_UNIT_BLOCKS 0x02000000 /* value is in blocks */ -- 2.34.1 --8KqcO45QTmavqrvZ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-WIP-test-GUC-default-values-in-postgresql.conf.sampl.patch" From 107e96e12dd25c8360470bd4b65f975925300d86 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Wed, 25 May 2022 05:14:43 -0500 Subject: [PATCH 2/2] WIP: test GUC default values in postgresql.conf.sample This tests for consistency between the default values written in postgresql.conf.sample and defined in guc.c. Kyotaro Horiguchi and Justin Pryzby //-os-only: freebsd, macos --- src/backend/utils/misc/guc.c | 80 +++++++++++++++ src/include/catalog/pg_proc.dat | 5 + src/test/modules/test_misc/t/003_check_guc.pl | 99 +++++++++++++++++-- .../unsafe_tests/expected/rolenames.out | 14 +++ .../modules/unsafe_tests/sql/rolenames.sql | 7 ++ 5 files changed, 199 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 53d1d9a06a7..9eef85a4fa4 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -3239,6 +3239,86 @@ parse_and_validate_value(struct config_generic *record, return true; } +/* + * Helper for pg_config_unitless_value + * + * Return the the value of the enum at the given index, or NULL if not found. + */ +static const char * +config_enum_lookup_by_value_soft(struct config_enum *record, int val) +{ + const struct config_enum_entry *entry; + + for (entry = record->options; entry && entry->name; entry++) + { + if (entry->val == val) + return entry->name; + } + + return NULL; +} + +/* + * Convert value to unitless value according to the specified GUC variable + */ +Datum +pg_config_unitless_value(PG_FUNCTION_ARGS) +{ + char *name; + char *value; + struct config_generic *record; + const char *result = ""; + void *extra; + union config_var_val val; + char buffer[256]; + + name = text_to_cstring(PG_GETARG_TEXT_PP(0)); + value = text_to_cstring(PG_GETARG_TEXT_PP(1)); + + record = find_option(name, true, false, ERROR); + + /* + * This function doesn't reveal values of the variables, but be consistent + * with similar functions. + */ + if ((record->flags & GUC_SUPERUSER_ONLY) && + !ConfigOptionIsVisible(record)) + ereport(ERROR, + errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to examine \"%s\"", name), + errdetail("Only roles with privileges of the \"%s\" role may examine this parameter.", + "pg_read_all_settings")); + + parse_and_validate_value(record, name, value, PGC_S_TEST, WARNING, + &val, &extra); + + switch (record->vartype) + { + case PGC_BOOL: + result = (val.boolval ? "on" : "off"); + break; + case PGC_INT: + snprintf(buffer, sizeof(buffer), "%d", val.intval); + result = buffer; + break; + case PGC_REAL: + snprintf(buffer, sizeof(buffer), "%g", val.realval); + result = buffer; + break; + case PGC_STRING: + result = val.stringval; + break; + case PGC_ENUM: + result = config_enum_lookup_by_value_soft((struct config_enum *) record, + val.intval); + break; + } + + if (result == NULL) + PG_RETURN_NULL(); + + PG_RETURN_TEXT_P(cstring_to_text(result)); +} /* * set_config_option: sets option `name' to given value. diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index aa293c43b6a..fe82629b1f6 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6242,6 +6242,11 @@ proname => 'pg_settings_get_flags', provolatile => 's', prorettype => '_text', proargtypes => 'text', prosrc => 'pg_settings_get_flags' }, +{ oid => '9956', descr => 'normalize value to the unit of specified GUC', + proname => 'pg_config_unitless_value', provolatile => 's', + prorettype => 'text', proargtypes => 'text text', + proargnames => '{varname,value}', prosrc => 'pg_config_unitless_value' }, + { oid => '3329', descr => 'show config file settings', proname => 'pg_show_all_file_settings', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', diff --git a/src/test/modules/test_misc/t/003_check_guc.pl b/src/test/modules/test_misc/t/003_check_guc.pl index e9f33f3c775..ce2ec1bf9ba 100644 --- a/src/test/modules/test_misc/t/003_check_guc.pl +++ b/src/test/modules/test_misc/t/003_check_guc.pl @@ -11,6 +11,26 @@ my $node = PostgreSQL::Test::Cluster->new('main'); $node->init; $node->start; +# These are non-variables but that are mistakenly parsed as variable +# settings in the loop below. +my %skip_names = + map { $_ => 1 } ('include', 'include_dir', 'include_if_exists'); + +# The following parameters have defaults which are +# environment-dependent and may not match the default +# values written in the sample config file. +my %ignore_parameters = + map { $_ => 1 } ( + 'data_directory', + 'hba_file', + 'ident_file', + 'krb_server_keyfile', + 'timezone_abbreviations', + 'lc_messages', + 'max_stack_depth', # XXX + 'wal_buffers', # XXX + ); + # Grab the names of all the parameters that can be listed in the # configuration sample file. config_file is an exception, it is not # in postgresql.conf.sample but is part of the lists from guc_tables.c. @@ -42,7 +62,7 @@ my @gucs_in_file; # Read the sample file line-by-line, checking its contents to build a list # of everything known as a GUC. -my $num_tests = 0; +my @file_vals = (); open(my $contents, '<', $sample_file) || die "Could not open $sample_file: $!"; while (my $line = <$contents>) @@ -52,19 +72,28 @@ while (my $line = <$contents>) # file. # - Valid configuration options are followed immediately by " = ", # with one space before and after the equal sign. - if ($line =~ m/^#?([_[:alpha:]]+) = .*/) + if ($line =~ m/^#?([_[:alpha:]]+) = (.*)$/) { # Lower-case conversion matters for some of the GUCs. my $param_name = lc($1); - # Ignore some exceptions. - next if $param_name eq "include"; - next if $param_name eq "include_dir"; - next if $param_name eq "include_if_exists"; + # extract value + my $file_value = $2; + $file_value =~ s/\s*#.*$//; # strip trailing comment + $file_value =~ s/^'(.*)'$/$1/; # strip quotes + + next if (defined $skip_names{$param_name}); # Update the list of GUCs found in the sample file, for the # follow-up tests. push @gucs_in_file, $param_name; + + # Update the list of GUCs whose value is checked for consistency + # between the sample file and pg_setting.boot_val + if (!defined $ignore_parameters{$param_name}) + { + push(@file_vals, [$param_name, $file_value]); + } } } @@ -106,4 +135,62 @@ foreach my $param (@sample_intersect) ); } +# Test that GUCs in postgresql.conf.sample show the correct default values +my $check_defaults = $node->safe_psql( + 'postgres', + " + CREATE TABLE sample_conf AS + SELECT m[1] AS name, COALESCE(m[3], m[5]) AS sample_value + FROM (SELECT regexp_split_to_table(pg_read_file('$sample_file'), '\n') AS ln) conf, + regexp_match(ln, '^#?([_[:alpha:]]+) (= ''([^'']*)''|(= ([^[:space:]]*))).*') AS m + WHERE ln ~ '^#?[[:alpha:]]' + "); + +$check_defaults = $node->safe_psql( + 'postgres', + " + SELECT name, sc.sample_value, boot_val + FROM pg_settings ps + JOIN sample_conf sc USING(name) + JOIN pg_settings_get_flags(ps.name) AS flags ON true + WHERE boot_val != sc.sample_value -- same value + AND boot_val != pg_config_unitless_value(name, sc.sample_value) -- same value with different units + AND 'DEFAULT_COMPILE' != ALL(flags) -- dynamically-set defaults + AND 'DEFAULT_INITDB' != ALL(flags) -- dynamically-set defaults + AND name NOT IN ('max_stack_depth', 'krb_server_keyfile'); -- exceptions + "); + +my @check_defaults_array = split("\n", lc($check_defaults)); + +is (@check_defaults_array, 0, + "check for consistency of defaults in postgresql.conf.sample: $check_defaults"); + +# XXX An alternative, perl implementation of the same thing: + +# Check if GUC values in config-file and boot value match +my $values = $node->safe_psql( + 'postgres', + 'SELECT f.n, pg_config_unitless_value(f.n, f.v), s.boot_val, \'!\' '. + 'FROM (VALUES '. + join(',', map { "('${$_}[0]','${$_}[1]')" } @file_vals). + ') f(n,v) '. + "JOIN pg_settings s ON (s.name = f.n)". + "JOIN pg_settings_get_flags(s.name) AS flags ON true ". + "AND 'DEFAULT_COMPILE' != ALL(flags) -- dynamically-set defaults ". + "AND 'DEFAULT_INITDB' != ALL(flags) -- dynamically-set defaults"); + +my $fails = ""; +foreach my $l (split("\n", $values)) +{ + # $l: |||! + my @t = split("\\|", $l); + if ($t[1] ne $t[2]) + { + $fails .= "\n" if ($fails ne ""); + $fails .= "$t[0]: file \"$t[1]\" != boot_val \"$t[2]\""; + } +} + +is($fails, "", "check if GUC values in .sample and boot value match"); + done_testing(); diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out index 61396b2a805..1bba9c87239 100644 --- a/src/test/modules/unsafe_tests/expected/rolenames.out +++ b/src/test/modules/unsafe_tests/expected/rolenames.out @@ -1082,6 +1082,20 @@ DETAIL: Only roles with privileges of the "pg_read_all_settings" role may exami RESET SESSION AUTHORIZATION; ERROR: current transaction is aborted, commands ignored until end of transaction block ROLLBACK; +BEGIN; +SET SESSION AUTHORIZATION regress_role_haspriv; +-- passes with role member of pg_read_all_settings +SELECT pg_config_unitless_value('session_preload_libraries', 'val'); + pg_config_unitless_value +-------------------------- + val +(1 row) + +SET SESSION AUTHORIZATION regress_role_nopriv; +SELECT pg_config_unitless_value('session_preload_libraries', 'val'); +ERROR: permission denied to examine "session_preload_libraries" +DETAIL: Only roles with privileges of the "pg_read_all_settings" role may examine this parameter. +ROLLBACK; REVOKE pg_read_all_settings FROM regress_role_haspriv; -- clean up \c diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql index adac36536db..355aa32c2ac 100644 --- a/src/test/modules/unsafe_tests/sql/rolenames.sql +++ b/src/test/modules/unsafe_tests/sql/rolenames.sql @@ -492,6 +492,13 @@ SET SESSION AUTHORIZATION regress_role_nopriv; SHOW session_preload_libraries; RESET SESSION AUTHORIZATION; ROLLBACK; +BEGIN; +SET SESSION AUTHORIZATION regress_role_haspriv; +-- passes with role member of pg_read_all_settings +SELECT pg_config_unitless_value('session_preload_libraries', 'val'); +SET SESSION AUTHORIZATION regress_role_nopriv; +SELECT pg_config_unitless_value('session_preload_libraries', 'val'); +ROLLBACK; REVOKE pg_read_all_settings FROM regress_role_haspriv; -- clean up -- 2.34.1 --8KqcO45QTmavqrvZ--