On Mon, Sep 26, 2022 at 05:29:58PM +0900, Kyotaro Horiguchi wrote: > At Sat, 17 Sep 2022 23:53:07 -0500, Justin Pryzby <pry...@telsasoft.com> > wrote in > > This is an alternative implementation, which still relies on adding the > > GUC_DYNAMIC, flag but doesn't require adding a new, sql-accessible > > function to convert the GUC to a pretty/human display value. > > Thanks! > > I'm not sure shared_buffer is GUC_DYNAMIC_DEFAULT, and we need to read
It's set during initdb. > postgresql.conf.sample using SQL, but +1 for the direction. > > + AND NOT (sc.sample_value ~ '^0' AND current_setting(name) ~ '^0') -- > zeros may be written differently > + AND NOT (sc.sample_value='60s' AND current_setting(name) = '1min') -- > two ways to write 1min > > Mmm. Couldn't we get away from that explicit exceptions? Suggestions are welcomed. Rebased the patch. I also split the flag into DEFAULTS_COMPILE and DEFAULTS_INITDB, since that makes it easier to understand what the flags mean and the intent of the patch. And maybe allows fewer exclusions in patches like Peter's, which I think would only want to exclude compile-time defaults. -- Justin
>From c3ac7bd40d26eb692f939d58935415ceb1cf308e Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Tue, 19 Jul 2022 08:31:56 -0500 Subject: [PATCH 1/2] add DYNAMIC_DEFAULT for settings which vary by ./configure or platform or initdb --- doc/src/sgml/func.sgml | 15 ++++++ src/backend/utils/misc/guc_funcs.c | 6 ++- src/backend/utils/misc/guc_tables.c | 78 ++++++++++++++++++----------- src/include/utils/guc.h | 2 + 4 files changed, 70 insertions(+), 31 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 6e0425cb3dc..b8e05073d1a 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -24374,6 +24374,21 @@ SELECT collation for ('foo' COLLATE "de_DE"); <row><entry>Flag</entry><entry>Description</entry></row> </thead> <tbody> + + <row> + <entry><literal>DEFAULT_COMPILE</literal></entry> + <entry>Parameters with this flag have default values which vary by + platform, or compile-time options. + </entry> + </row> + + <row> + <entry><literal>DEFAULT_INITDB</literal></entry> + <entry>Parameters with this flag have default values which are + determined dynamically during initdb. + </entry> + </row> + <row> <entry><literal>EXPLAIN</literal></entry> <entry>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 108b3bd1290..2b666e8d014 100644 --- a/src/backend/utils/misc/guc_funcs.c +++ b/src/backend/utils/misc/guc_funcs.c @@ -538,7 +538,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; @@ -551,6 +551,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 6454d034e7e..885d3dd8b81 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -601,7 +601,7 @@ StaticAssertDecl(lengthof(GucContext_Names) == (PGC_USERSET + 1), const char *const GucSource_Names[] = { /* PGC_S_DEFAULT */ "default", - /* PGC_S_DYNAMIC_DEFAULT */ "default", + /* PGC_S_DYNAMIC_DEFAULT */ "default", // /* PGC_S_ENV_VAR */ "environment variable", /* PGC_S_FILE */ "configuration file", /* PGC_S_ARGV */ "command line", @@ -1191,7 +1191,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, #ifdef USE_ASSERT_CHECKING @@ -1367,7 +1367,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, #ifdef WIN32 @@ -2116,7 +2117,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, @@ -2153,7 +2155,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, @@ -2196,7 +2198,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, @@ -2225,7 +2228,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, @@ -2667,7 +2671,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, @@ -2885,7 +2889,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, @@ -2898,7 +2902,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, #ifdef USE_PREFETCH @@ -2916,7 +2920,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, #ifdef USE_PREFETCH @@ -2933,7 +2937,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, @@ -3387,7 +3391,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 @@ -3880,7 +3884,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", @@ -3892,7 +3897,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", @@ -3994,7 +3999,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, "", @@ -4004,7 +4010,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", @@ -4014,7 +4021,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", @@ -4024,7 +4032,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", @@ -4183,7 +4192,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", @@ -4214,7 +4224,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, @@ -4295,7 +4305,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 @@ -4370,7 +4380,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", @@ -4381,7 +4393,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 @@ -4396,7 +4408,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 @@ -4634,7 +4646,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, #ifdef HAVE_SYSLOG @@ -4747,7 +4760,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, @@ -4757,7 +4772,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, @@ -4767,7 +4783,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, @@ -4829,7 +4846,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 b3aaff9665b..79053583a03 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.25.1
>From 862586ef48b35071393c858c01419144d09c8400 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Wed, 25 May 2022 05:14:43 -0500 Subject: [PATCH 2/2] WIP: test guc default values --- src/backend/utils/misc/postgresql.conf.sample | 2 +- src/test/modules/test_misc/t/003_check_guc.pl | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 1ce0dee6d04..21bfbef3291 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -720,7 +720,7 @@ # share/timezonesets/. #extra_float_digits = 1 # min -15, max 3; any value >0 actually # selects precise output mode -#client_encoding = sql_ascii # actually, defaults to database +#client_encoding = SQL_ASCII # actually, defaults to database # encoding # These settings are initialized by initdb, but they can be changed. 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 60459ef759e..a660e85db58 100644 --- a/src/test/modules/test_misc/t/003_check_guc.pl +++ b/src/test/modules/test_misc/t/003_check_guc.pl @@ -107,4 +107,37 @@ 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, current_setting(name), sc.sample_value, boot_val + FROM pg_show_all_settings() psas + JOIN sample_conf sc USING(name) + WHERE sc.sample_value != boot_val -- same value (specified by Cluster.pm) + AND sc.sample_value != current_setting(name) -- same value, with human units + AND sc.sample_value != current_setting(name)||'.0' -- same value with .0 suffix + AND 'DEFAULT_COMPILE' != ALL(pg_settings_get_flags(psas.name)) -- dynamically-set defaults + AND 'DEFAULT_INITDB' != ALL(pg_settings_get_flags(psas.name)) -- dynamically-set defaults + AND NOT (sc.sample_value ~ '^0' AND current_setting(name) ~ '^0') -- zeros may be written differently + AND NOT (sc.sample_value='60s' AND current_setting(name) = '1min') -- two ways to write 1min + AND name NOT IN ('krb_server_keyfile', 'wal_retrieve_retry_interval', 'log_autovacuum_min_duration'); -- exceptions + "); + +my @check_defaults_array = split("\n", lc($check_defaults)); +print(STDERR "$check_defaults\n"); + +is (@check_defaults_array, 0, + 'check for consistency of defaults in postgresql.conf.sample'); + done_testing(); -- 2.25.1