public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v3 2/2] warn when setting GUC to a nonextant library
11+ messages / 5 participants
[nested] [flat]
* [PATCH v3 2/2] warn when setting GUC to a nonextant library
@ 2021-12-13 14:42 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Justin Pryzby @ 2021-12-13 14:42 UTC (permalink / raw)
XXX: the SPI test is unstable:
-SELECT * FROM schema4.counted;
WARNING: could not load library: $libdir/plugins/worker_spi: cannot open shared object file: No such file or directory
+SELECT * FROM schema4.counted;
---
src/backend/utils/misc/guc.c | 85 ++++++++++++++++++-
.../unsafe_tests/expected/rolenames.out | 1 +
.../worker_spi/expected/worker_spi.out | 2 +
.../modules/worker_spi/sql/worker_spi.sql | 2 +
src/test/regress/expected/rules.out | 5 ++
src/test/regress/sql/rules.sql | 1 +
6 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4c94f09c645..5a228a37ee2 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -170,6 +170,12 @@ static bool call_enum_check_hook(struct config_enum *conf, int *newval,
void **extra, GucSource source, int elevel);
static bool check_log_destination(char **newval, void **extra, GucSource source);
+static bool check_local_preload_libraries(char **newval, void **extra,
+ GucSource source);
+static bool check_session_preload_libraries(char **newval, void **extra,
+ GucSource source);
+static bool check_shared_preload_libraries(char **newval, void **extra,
+ GucSource source);
static void assign_log_destination(const char *newval, void *extra);
static bool check_wal_consistency_checking(char **newval, void **extra,
@@ -4188,7 +4194,7 @@ static struct config_string ConfigureNamesString[] =
},
&session_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_session_preload_libraries, NULL, NULL
},
{
@@ -4199,7 +4205,7 @@ static struct config_string ConfigureNamesString[] =
},
&shared_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_shared_preload_libraries, NULL, NULL
},
{
@@ -4210,7 +4216,7 @@ static struct config_string ConfigureNamesString[] =
},
&local_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_local_preload_libraries, NULL, NULL
},
{
@@ -12100,6 +12106,79 @@ check_max_worker_processes(int *newval, void **extra, GucSource source)
return true;
}
+/*
+ * See also load_libraries() and internal_load_library().
+ */
+static bool
+check_preload_libraries(char **newval, void **extra, GucSource source,
+ bool restricted)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ /* nothing to do if empty */
+ if (newval == NULL || *newval[0] == '\0')
+ return true;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of filename paths */
+ if (!SplitDirectoriesString(rawstring, ',', &elemlist))
+ {
+ /* Should not happen ? */
+ return false;
+ }
+
+ foreach(l, elemlist)
+ {
+ /* Note that filename was already canonicalized */
+ char *filename = (char *) lfirst(l);
+ char *expanded = NULL;
+
+ /* If restricting, insert $libdir/plugins if not mentioned already */
+ if (restricted && first_dir_separator(filename) == NULL)
+ {
+ expanded = psprintf("$libdir/plugins/%s", filename);
+ filename = expanded;
+ }
+
+ /*
+ * stat()/access() only check that the library exists, not that it has
+ * the correct magic number or even a library. But error messages from
+ * dlopen() are not portable, so it'd be hard to report any problem
+ * other than "does not exist".
+ */
+ if (access(filename, R_OK) == -1) // F_OK
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename));
+ }
+
+ list_free_deep(elemlist);
+ pfree(rawstring);
+ return true;
+}
+
+static bool
+check_shared_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
+
+static bool
+check_local_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, false);
+}
+
+static bool
+check_session_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
+
static bool
check_effective_io_concurrency(int *newval, void **extra, GucSource source)
{
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index eb608fdc2ea..368b10558d7 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1066,6 +1066,7 @@ GRANT pg_read_all_settings TO regress_role_haspriv;
BEGIN;
-- A GUC using GUC_SUPERUSER_ONLY is useful for negative tests.
SET LOCAL session_preload_libraries TO 'path-to-preload-libraries';
+WARNING: could not access file "$libdir/plugins/path-to-preload-libraries"
SET SESSION AUTHORIZATION regress_role_haspriv;
-- passes with role member of pg_read_all_settings
SHOW session_preload_libraries;
diff --git a/src/test/modules/worker_spi/expected/worker_spi.out b/src/test/modules/worker_spi/expected/worker_spi.out
index dc0a79bf759..5b275669bcf 100644
--- a/src/test/modules/worker_spi/expected/worker_spi.out
+++ b/src/test/modules/worker_spi/expected/worker_spi.out
@@ -28,6 +28,8 @@ SELECT pg_reload_conf();
t
(1 row)
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/modules/worker_spi/sql/worker_spi.sql b/src/test/modules/worker_spi/sql/worker_spi.sql
index 4683523b29d..4ed5370d456 100644
--- a/src/test/modules/worker_spi/sql/worker_spi.sql
+++ b/src/test/modules/worker_spi/sql/worker_spi.sql
@@ -18,6 +18,8 @@ END
$$;
INSERT INTO schema4.counted VALUES ('total', 0), ('delta', 1);
SELECT pg_reload_conf();
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index d652f7b5fb4..ce396814332 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3574,6 +3574,7 @@ drop table hats;
drop table hat_data;
-- test for pg_get_functiondef properly regurgitating SET parameters
-- Note that the function is kept around to stress pg_dump.
+SET check_function_bodies=no;
CREATE FUNCTION func_with_set_params() RETURNS integer
AS 'select 1;'
LANGUAGE SQL
@@ -3583,6 +3584,10 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
SET datestyle to iso, mdy
SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
IMMUTABLE STRICT;
+WARNING: could not access file "Mixed/Case"
+WARNING: could not access file "c:/'a"/path"
+WARNING: could not access file ""
+WARNING: could not access file "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
pg_get_functiondef
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index b732833e63c..1e42d092862 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1198,6 +1198,7 @@ drop table hat_data;
-- test for pg_get_functiondef properly regurgitating SET parameters
-- Note that the function is kept around to stress pg_dump.
+SET check_function_bodies=no;
CREATE FUNCTION func_with_set_params() RETURNS integer
AS 'select 1;'
LANGUAGE SQL
--
2.17.1
--9Q79yvCDNTHf0qBd--
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v6 2/4] warn when setting GUC to a nonextant library
@ 2021-12-13 14:42 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Justin Pryzby @ 2021-12-13 14:42 UTC (permalink / raw)
TODO: test that warnings aren't issued during startup (only fatals)
---
src/backend/commands/variable.c | 96 +++++++++++++++++++
src/backend/utils/misc/guc.c | 2 +-
src/backend/utils/misc/guc_tables.c | 6 +-
src/include/utils/guc_hooks.h | 7 ++
.../unsafe_tests/expected/rolenames.out | 19 ++++
.../modules/unsafe_tests/sql/rolenames.sql | 13 +++
.../worker_spi/expected/worker_spi.out | 2 +
.../modules/worker_spi/sql/worker_spi.sql | 2 +
src/test/regress/expected/rules.out | 8 ++
9 files changed, 151 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 791bac6715c..85642ab8453 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -40,6 +40,7 @@
#include "utils/timestamp.h"
#include "utils/tzparser.h"
#include "utils/varlena.h"
+#include <unistd.h>
/*
* DATESTYLE
@@ -1210,3 +1211,98 @@ check_ssl(bool *newval, void **extra, GucSource source)
#endif
return true;
}
+
+
+/*
+ * See also load_libraries() and internal_load_library().
+ */
+static bool
+check_preload_libraries(char **newval, void **extra, GucSource source,
+ bool restricted)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ /* nothing to do if empty */
+ if (newval == NULL || *newval[0] == '\0')
+ return true;
+
+ /*
+ * issue warnings only during an interactive SET, from ALTER
+ * ROLE/DATABASE/SYSTEM.
+ */
+ if (source != PGC_S_TEST)
+ return true;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of filename paths */
+ if (!SplitDirectoriesString(rawstring, ',', &elemlist))
+ {
+ /* Should not happen ? */
+ return false;
+ }
+
+ foreach(l, elemlist)
+ {
+ /* Note that filename was already canonicalized */
+ char *filename = (char *) lfirst(l);
+ char *expanded = NULL;
+
+ /* If restricting, insert $libdir/plugins if not mentioned already */
+ if (restricted && first_dir_separator(filename) == NULL)
+ {
+ expanded = psprintf("$libdir/plugins/%s", filename);
+ filename = expanded;
+ }
+
+ /*
+ * stat()/access() only check that the library exists, not that it has
+ * the correct magic number or even a library. But error messages from
+ * dlopen() are not portable, so it'd be hard to report any problem
+ * other than "does not exist".
+ */
+ if (access(filename, R_OK) == 0)
+ continue;
+
+
+ if (source == PGC_S_FILE)
+ /* ALTER SYSTEM */
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errdetail("The server will currently fail to start with this setting."),
+ errhint("If the server is shut down, it will be necessary to manually edit the %s file to allow it to start again.",
+ "postgresql.auto.conf"));
+ else
+ /* ALTER ROLE/DATABASE */
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errdetail("New sessions will currently fail to connect with the new setting."));
+ }
+
+ list_free_deep(elemlist);
+ pfree(rawstring);
+ return true;
+}
+
+bool
+check_shared_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
+
+bool
+check_local_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, false);
+}
+
+bool
+check_session_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ae8810591d6..9c284906d6e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -24,7 +24,7 @@
*/
#include "postgres.h"
-#include <limits.h>
+// #include <limits.h>
#include <sys/stat.h>
#include <unistd.h>
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 05ab087934c..b7c8d8dcdfb 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4015,7 +4015,7 @@ struct config_string ConfigureNamesString[] =
},
&session_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_session_preload_libraries, NULL, NULL
},
{
@@ -4026,7 +4026,7 @@ struct config_string ConfigureNamesString[] =
},
&shared_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_shared_preload_libraries, NULL, NULL
},
{
@@ -4037,7 +4037,7 @@ struct config_string ConfigureNamesString[] =
},
&local_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_local_preload_libraries, NULL, NULL
},
{
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index f1a9a183b49..3f51436a6f5 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -155,4 +155,11 @@ extern bool check_wal_consistency_checking(char **newval, void **extra,
extern void assign_wal_consistency_checking(const char *newval, void *extra);
extern void assign_xlog_sync_method(int new_sync_method, void *extra);
+extern bool check_local_preload_libraries(char **newval, void **extra,
+ GucSource source);
+extern bool check_session_preload_libraries(char **newval, void **extra,
+ GucSource source);
+extern bool check_shared_preload_libraries(char **newval, void **extra,
+ GucSource source);
+
#endif /* GUC_HOOKS_H */
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 88b1ff843be..ac161ce7c33 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1082,6 +1082,25 @@ RESET SESSION AUTHORIZATION;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ROLLBACK;
REVOKE pg_read_all_settings FROM regress_role_haspriv;
+-- test some warnings
+ALTER ROLE regress_testrol0 SET session_preload_libraries="DoesNotExist";
+WARNING: could not access file "$libdir/plugins/DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+ALTER ROLE regress_testrol0 SET local_preload_libraries="DoesNotExist";
+WARNING: could not access file "DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+CREATE DATABASE regress_nosuch_db;
+ALTER DATABASE regress_nosuch_db SET session_preload_libraries="DoesNotExist";
+WARNING: could not access file "$libdir/plugins/DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+ALTER DATABASE regress_nosuch_db SET local_preload_libraries="DoesNotExist";
+WARNING: could not access file "DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+DROP DATABASE regress_nosuch_db;
+-- SET doesn't do anything, but should not warn, either
+SET session_preload_libraries="DoesNotExist";
+SET SESSION session_preload_libraries="DoesNotExist";
+begin; SET LOCAL session_preload_libraries="DoesNotExist"; rollback;
-- clean up
\c
DROP SCHEMA test_roles_schema;
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..cf605b08d30 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -494,6 +494,19 @@ RESET SESSION AUTHORIZATION;
ROLLBACK;
REVOKE pg_read_all_settings FROM regress_role_haspriv;
+-- test some warnings
+ALTER ROLE regress_testrol0 SET session_preload_libraries="DoesNotExist";
+ALTER ROLE regress_testrol0 SET local_preload_libraries="DoesNotExist";
+CREATE DATABASE regress_nosuch_db;
+ALTER DATABASE regress_nosuch_db SET session_preload_libraries="DoesNotExist";
+ALTER DATABASE regress_nosuch_db SET local_preload_libraries="DoesNotExist";
+DROP DATABASE regress_nosuch_db;
+
+-- SET doesn't do anything, but should not warn, either
+SET session_preload_libraries="DoesNotExist";
+SET SESSION session_preload_libraries="DoesNotExist";
+begin; SET LOCAL session_preload_libraries="DoesNotExist"; rollback;
+
-- clean up
\c
diff --git a/src/test/modules/worker_spi/expected/worker_spi.out b/src/test/modules/worker_spi/expected/worker_spi.out
index dc0a79bf759..5b275669bcf 100644
--- a/src/test/modules/worker_spi/expected/worker_spi.out
+++ b/src/test/modules/worker_spi/expected/worker_spi.out
@@ -28,6 +28,8 @@ SELECT pg_reload_conf();
t
(1 row)
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/modules/worker_spi/sql/worker_spi.sql b/src/test/modules/worker_spi/sql/worker_spi.sql
index 4683523b29d..4ed5370d456 100644
--- a/src/test/modules/worker_spi/sql/worker_spi.sql
+++ b/src/test/modules/worker_spi/sql/worker_spi.sql
@@ -18,6 +18,8 @@ END
$$;
INSERT INTO schema4.counted VALUES ('total', 0), ('delta', 1);
SELECT pg_reload_conf();
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 624d0e5aae1..3a36898b8d3 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3398,6 +3398,14 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
SET datestyle to iso, mdy
SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
IMMUTABLE STRICT;
+WARNING: could not access file "Mixed/Case"
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file "c:/'a"/path"
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file ""
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
+DETAIL: New sessions will currently fail to connect with the new setting.
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
pg_get_functiondef
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--
2.25.1
--vDrIcvfimaPJgjx+
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0003-errcontext-if-server-fails-to-start-due-to-librar.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH 2/4] warn when setting GUC to a nonextant library
@ 2021-12-13 14:42 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Justin Pryzby @ 2021-12-13 14:42 UTC (permalink / raw)
Note that warnings shouldn't be issued during startup (only fatals):
$ ./tmp_install/usr/local/pgsql/bin/postgres -D ./testrun/regress/regress/tmp_check/data -c shared_preload_libraries=ab
2023-07-06 14:47:03.817 CDT postmaster[2608106] FATAL: could not access file "ab": No existe el archivo o el directorio
2023-07-06 14:47:03.817 CDT postmaster[2608106] CONTEXT: while loading shared libraries for setting "shared_preload_libraries"
---
src/backend/commands/variable.c | 95 +++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 6 +-
src/include/utils/guc_hooks.h | 7 ++
.../unsafe_tests/expected/rolenames.out | 19 ++++
.../modules/unsafe_tests/sql/rolenames.sql | 13 +++
.../worker_spi/expected/worker_spi.out | 2 +
.../modules/worker_spi/sql/worker_spi.sql | 2 +
src/test/regress/expected/rules.out | 8 ++
8 files changed, 149 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index f0f2e076552..9272a0f0e5a 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -40,6 +40,7 @@
#include "utils/timestamp.h"
#include "utils/tzparser.h"
#include "utils/varlena.h"
+#include <unistd.h>
/*
* DATESTYLE
@@ -1210,3 +1211,97 @@ check_ssl(bool *newval, void **extra, GucSource source)
#endif
return true;
}
+
+
+/*
+ * See also load_libraries() and internal_load_library().
+ */
+static bool
+check_preload_libraries(char **newval, void **extra, GucSource source,
+ bool restricted)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ /* nothing to do if empty */
+ if (newval == NULL || *newval[0] == '\0')
+ return true;
+
+ /*
+ * issue warnings only during an interactive SET, from ALTER
+ * ROLE/DATABASE/SYSTEM.
+ */
+ if (source != PGC_S_TEST)
+ return true;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of filename paths */
+ if (!SplitDirectoriesString(rawstring, ',', &elemlist))
+ {
+ /* Should not happen ? */
+ return false;
+ }
+
+ foreach(l, elemlist)
+ {
+ /* Note that filename was already canonicalized */
+ char *filename = (char *) lfirst(l);
+ char *expanded = NULL;
+
+ /* If restricting, insert $libdir/plugins if not mentioned already */
+ if (restricted && first_dir_separator(filename) == NULL)
+ {
+ expanded = psprintf("$libdir/plugins/%s", filename);
+ filename = expanded;
+ }
+
+ /*
+ * stat()/access() only check that the library exists, not that it has
+ * the correct magic number or even a library. But error messages
+ * from dlopen() are not portable, so it'd be hard to report any
+ * problem other than "does not exist".
+ */
+ if (access(filename, R_OK) == 0)
+ continue;
+
+ if (source == PGC_S_FILE)
+ /* ALTER SYSTEM */
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errdetail("The server will currently fail to start with this setting."),
+ errhint("If the server is shut down, it will be necessary to manually edit the %s file to allow it to start again.",
+ "postgresql.auto.conf"));
+ else
+ /* ALTER ROLE/DATABASE */
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errdetail("New sessions will currently fail to connect with the new setting."));
+ }
+
+ list_free_deep(elemlist);
+ pfree(rawstring);
+ return true;
+}
+
+bool
+check_shared_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
+
+bool
+check_local_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, false);
+}
+
+bool
+check_session_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 59ab630ae40..e4a80d87928 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4101,7 +4101,7 @@ struct config_string ConfigureNamesString[] =
},
&session_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_session_preload_libraries, NULL, NULL
},
{
@@ -4112,7 +4112,7 @@ struct config_string ConfigureNamesString[] =
},
&shared_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_shared_preload_libraries, NULL, NULL
},
{
@@ -4123,7 +4123,7 @@ struct config_string ConfigureNamesString[] =
},
&local_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_local_preload_libraries, NULL, NULL
},
{
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 2ecb9fc0866..de030d750f1 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -160,4 +160,11 @@ extern void assign_xlog_sync_method(int new_sync_method, void *extra);
extern bool check_io_direct(char **newval, void **extra, GucSource source);
extern void assign_io_direct(const char *newval, void *extra);
+extern bool check_local_preload_libraries(char **newval, void **extra,
+ GucSource source);
+extern bool check_session_preload_libraries(char **newval, void **extra,
+ GucSource source);
+extern bool check_shared_preload_libraries(char **newval, void **extra,
+ GucSource source);
+
#endif /* GUC_HOOKS_H */
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 61396b2a805..2dcd7cf9ec0 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1083,6 +1083,25 @@ RESET SESSION AUTHORIZATION;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ROLLBACK;
REVOKE pg_read_all_settings FROM regress_role_haspriv;
+-- test some warnings
+ALTER ROLE regress_testrol0 SET session_preload_libraries="DoesNotExist";
+WARNING: could not access file "$libdir/plugins/DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+ALTER ROLE regress_testrol0 SET local_preload_libraries="DoesNotExist";
+WARNING: could not access file "DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+CREATE DATABASE regression_nosuch_db;
+ALTER DATABASE regression_nosuch_db SET session_preload_libraries="DoesNotExist";
+WARNING: could not access file "$libdir/plugins/DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+ALTER DATABASE regression_nosuch_db SET local_preload_libraries="DoesNotExist";
+WARNING: could not access file "DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+DROP DATABASE regression_nosuch_db;
+-- SET doesn't do anything, but should not warn, either
+SET session_preload_libraries="DoesNotExist";
+SET SESSION session_preload_libraries="DoesNotExist";
+begin; SET LOCAL session_preload_libraries="DoesNotExist"; rollback;
-- clean up
\c
DROP SCHEMA test_roles_schema;
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..56fb52d4e08 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -494,6 +494,19 @@ RESET SESSION AUTHORIZATION;
ROLLBACK;
REVOKE pg_read_all_settings FROM regress_role_haspriv;
+-- test some warnings
+ALTER ROLE regress_testrol0 SET session_preload_libraries="DoesNotExist";
+ALTER ROLE regress_testrol0 SET local_preload_libraries="DoesNotExist";
+CREATE DATABASE regression_nosuch_db;
+ALTER DATABASE regression_nosuch_db SET session_preload_libraries="DoesNotExist";
+ALTER DATABASE regression_nosuch_db SET local_preload_libraries="DoesNotExist";
+DROP DATABASE regression_nosuch_db;
+
+-- SET doesn't do anything, but should not warn, either
+SET session_preload_libraries="DoesNotExist";
+SET SESSION session_preload_libraries="DoesNotExist";
+begin; SET LOCAL session_preload_libraries="DoesNotExist"; rollback;
+
-- clean up
\c
diff --git a/src/test/modules/worker_spi/expected/worker_spi.out b/src/test/modules/worker_spi/expected/worker_spi.out
index dc0a79bf759..5b275669bcf 100644
--- a/src/test/modules/worker_spi/expected/worker_spi.out
+++ b/src/test/modules/worker_spi/expected/worker_spi.out
@@ -28,6 +28,8 @@ SELECT pg_reload_conf();
t
(1 row)
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/modules/worker_spi/sql/worker_spi.sql b/src/test/modules/worker_spi/sql/worker_spi.sql
index 4683523b29d..4ed5370d456 100644
--- a/src/test/modules/worker_spi/sql/worker_spi.sql
+++ b/src/test/modules/worker_spi/sql/worker_spi.sql
@@ -18,6 +18,8 @@ END
$$;
INSERT INTO schema4.counted VALUES ('total', 0), ('delta', 1);
SELECT pg_reload_conf();
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 7fd81e6a7d0..299c9cb4d2d 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3453,6 +3453,14 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
SET datestyle to iso, mdy
SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
IMMUTABLE STRICT;
+WARNING: could not access file "Mixed/Case"
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file "c:/'a"/path"
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file ""
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
+DETAIL: New sessions will currently fail to connect with the new setting.
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
pg_get_functiondef
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--
2.34.1
--Cz2/h24zxIeGl+Za
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0003-errcontext-if-server-fails-to-start-due-to-library-G.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v5 1/3] warn when setting GUC to a nonextant library
@ 2021-12-13 14:42 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Justin Pryzby @ 2021-12-13 14:42 UTC (permalink / raw)
---
src/backend/utils/misc/guc.c | 106 +++++++++++++++++-
.../unsafe_tests/expected/rolenames.out | 19 ++++
.../modules/unsafe_tests/sql/rolenames.sql | 13 +++
.../worker_spi/expected/worker_spi.out | 2 +
.../modules/worker_spi/sql/worker_spi.sql | 2 +
src/test/regress/expected/rules.out | 8 ++
6 files changed, 147 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index af4a1c30689..4e38e73a277 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -176,6 +176,12 @@ static bool call_enum_check_hook(struct config_enum *conf, int *newval,
void **extra, GucSource source, int elevel);
static bool check_log_destination(char **newval, void **extra, GucSource source);
+static bool check_local_preload_libraries(char **newval, void **extra,
+ GucSource source);
+static bool check_session_preload_libraries(char **newval, void **extra,
+ GucSource source);
+static bool check_shared_preload_libraries(char **newval, void **extra,
+ GucSource source);
static void assign_log_destination(const char *newval, void *extra);
static bool check_wal_consistency_checking(char **newval, void **extra,
@@ -4272,7 +4278,7 @@ static struct config_string ConfigureNamesString[] =
},
&session_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_session_preload_libraries, NULL, NULL
},
{
@@ -4283,7 +4289,7 @@ static struct config_string ConfigureNamesString[] =
},
&shared_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_shared_preload_libraries, NULL, NULL
},
{
@@ -4294,7 +4300,7 @@ static struct config_string ConfigureNamesString[] =
},
&local_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_local_preload_libraries, NULL, NULL
},
{
@@ -12506,6 +12512,100 @@ check_max_worker_processes(int *newval, void **extra, GucSource source)
return true;
}
+/*
+ * See also load_libraries() and internal_load_library().
+ */
+static bool
+check_preload_libraries(char **newval, void **extra, GucSource source,
+ bool restricted)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ /* nothing to do if empty */
+ if (newval == NULL || *newval[0] == '\0')
+ return true;
+
+ /*
+ * issue warnings only during an interactive SET, from ALTER
+ * ROLE/DATABASE/SYSTEM.
+ */
+ if (source != PGC_S_TEST && source != PGC_S_FILE)
+ return true;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of filename paths */
+ if (!SplitDirectoriesString(rawstring, ',', &elemlist))
+ {
+ /* Should not happen ? */
+ return false;
+ }
+
+ foreach(l, elemlist)
+ {
+ /* Note that filename was already canonicalized */
+ char *filename = (char *) lfirst(l);
+ char *expanded = NULL;
+
+ /* If restricting, insert $libdir/plugins if not mentioned already */
+ if (restricted && first_dir_separator(filename) == NULL)
+ {
+ expanded = psprintf("$libdir/plugins/%s", filename);
+ filename = expanded;
+ }
+
+ /*
+ * stat()/access() only check that the library exists, not that it has
+ * the correct magic number or even a library. But error messages from
+ * dlopen() are not portable, so it'd be hard to report any problem
+ * other than "does not exist".
+ */
+ if (access(filename, R_OK) == 0)
+ continue;
+
+
+ if (source == PGC_S_FILE)
+ /* ALTER SYSTEM */
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errdetail("The server will currently fail to start with this setting."),
+ errhint("If the server is shut down, it will be necessary to manually edit the %s file to allow it to start again.",
+ "postgresql.auto.conf"));
+ else
+ /* ALTER ROLE/DATABASE */
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errdetail("New sessions will currently fail to connect with the new setting."));
+ }
+
+ list_free_deep(elemlist);
+ pfree(rawstring);
+ return true;
+}
+
+static bool
+check_shared_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
+
+static bool
+check_local_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, false);
+}
+
+static bool
+check_session_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true);
+}
+
static bool
check_effective_io_concurrency(int *newval, void **extra, GucSource source)
{
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 88b1ff843be..ac161ce7c33 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1082,6 +1082,25 @@ RESET SESSION AUTHORIZATION;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ROLLBACK;
REVOKE pg_read_all_settings FROM regress_role_haspriv;
+-- test some warnings
+ALTER ROLE regress_testrol0 SET session_preload_libraries="DoesNotExist";
+WARNING: could not access file "$libdir/plugins/DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+ALTER ROLE regress_testrol0 SET local_preload_libraries="DoesNotExist";
+WARNING: could not access file "DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+CREATE DATABASE regress_nosuch_db;
+ALTER DATABASE regress_nosuch_db SET session_preload_libraries="DoesNotExist";
+WARNING: could not access file "$libdir/plugins/DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+ALTER DATABASE regress_nosuch_db SET local_preload_libraries="DoesNotExist";
+WARNING: could not access file "DoesNotExist"
+DETAIL: New sessions will currently fail to connect with the new setting.
+DROP DATABASE regress_nosuch_db;
+-- SET doesn't do anything, but should not warn, either
+SET session_preload_libraries="DoesNotExist";
+SET SESSION session_preload_libraries="DoesNotExist";
+begin; SET LOCAL session_preload_libraries="DoesNotExist"; rollback;
-- clean up
\c
DROP SCHEMA test_roles_schema;
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..cf605b08d30 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -494,6 +494,19 @@ RESET SESSION AUTHORIZATION;
ROLLBACK;
REVOKE pg_read_all_settings FROM regress_role_haspriv;
+-- test some warnings
+ALTER ROLE regress_testrol0 SET session_preload_libraries="DoesNotExist";
+ALTER ROLE regress_testrol0 SET local_preload_libraries="DoesNotExist";
+CREATE DATABASE regress_nosuch_db;
+ALTER DATABASE regress_nosuch_db SET session_preload_libraries="DoesNotExist";
+ALTER DATABASE regress_nosuch_db SET local_preload_libraries="DoesNotExist";
+DROP DATABASE regress_nosuch_db;
+
+-- SET doesn't do anything, but should not warn, either
+SET session_preload_libraries="DoesNotExist";
+SET SESSION session_preload_libraries="DoesNotExist";
+begin; SET LOCAL session_preload_libraries="DoesNotExist"; rollback;
+
-- clean up
\c
diff --git a/src/test/modules/worker_spi/expected/worker_spi.out b/src/test/modules/worker_spi/expected/worker_spi.out
index dc0a79bf759..5b275669bcf 100644
--- a/src/test/modules/worker_spi/expected/worker_spi.out
+++ b/src/test/modules/worker_spi/expected/worker_spi.out
@@ -28,6 +28,8 @@ SELECT pg_reload_conf();
t
(1 row)
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/modules/worker_spi/sql/worker_spi.sql b/src/test/modules/worker_spi/sql/worker_spi.sql
index 4683523b29d..4ed5370d456 100644
--- a/src/test/modules/worker_spi/sql/worker_spi.sql
+++ b/src/test/modules/worker_spi/sql/worker_spi.sql
@@ -18,6 +18,8 @@ END
$$;
INSERT INTO schema4.counted VALUES ('total', 0), ('delta', 1);
SELECT pg_reload_conf();
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 7ec3d2688f0..ea4f9eb39e2 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3392,6 +3392,14 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
SET datestyle to iso, mdy
SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
IMMUTABLE STRICT;
+WARNING: could not access file "Mixed/Case"
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file "c:/'a"/path"
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file ""
+DETAIL: New sessions will currently fail to connect with the new setting.
+WARNING: could not access file "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
+DETAIL: New sessions will currently fail to connect with the new setting.
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
pg_get_functiondef
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--
2.17.1
--3siQDZowHQqNOShm
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-errcontext-if-server-fails-to-start-due-to-librar.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v4 1/3] warn when setting GUC to a nonextant library
@ 2021-12-13 14:42 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Justin Pryzby @ 2021-12-13 14:42 UTC (permalink / raw)
---
src/backend/utils/misc/guc.c | 103 +++++++++++++++++-
.../unsafe_tests/expected/rolenames.out | 1 +
.../worker_spi/expected/worker_spi.out | 2 +
.../modules/worker_spi/sql/worker_spi.sql | 2 +
src/test/regress/expected/rules.out | 9 ++
src/test/regress/sql/rules.sql | 1 +
6 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f505413a7f9..c44b8ebbfd6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -170,6 +170,12 @@ static bool call_enum_check_hook(struct config_enum *conf, int *newval,
void **extra, GucSource source, int elevel);
static bool check_log_destination(char **newval, void **extra, GucSource source);
+static bool check_local_preload_libraries(char **newval, void **extra,
+ GucSource source);
+static bool check_session_preload_libraries(char **newval, void **extra,
+ GucSource source);
+static bool check_shared_preload_libraries(char **newval, void **extra,
+ GucSource source);
static void assign_log_destination(const char *newval, void *extra);
static bool check_wal_consistency_checking(char **newval, void **extra,
@@ -4198,7 +4204,7 @@ static struct config_string ConfigureNamesString[] =
},
&session_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_session_preload_libraries, NULL, NULL
},
{
@@ -4209,7 +4215,7 @@ static struct config_string ConfigureNamesString[] =
},
&shared_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_shared_preload_libraries, NULL, NULL
},
{
@@ -4220,7 +4226,7 @@ static struct config_string ConfigureNamesString[] =
},
&local_preload_libraries_string,
"",
- NULL, NULL, NULL
+ check_local_preload_libraries, NULL, NULL
},
{
@@ -12149,6 +12155,97 @@ check_max_worker_processes(int *newval, void **extra, GucSource source)
return true;
}
+/*
+ * See also load_libraries() and internal_load_library().
+ */
+static bool
+check_preload_libraries(char **newval, void **extra, GucSource source,
+ bool restricted, bool is_session)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ /* nothing to do if empty */
+ if (newval == NULL || *newval[0] == '\0')
+ return true;
+
+ /*
+ * Do not issue warnings while applying GUCs during startup. That would
+ * issue a FATAL error message, and the warnings would be redundant.
+ */
+ if (!IsUnderPostmaster)
+ return true;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of filename paths */
+ if (!SplitDirectoriesString(rawstring, ',', &elemlist))
+ {
+ /* Should not happen ? */
+ return false;
+ }
+
+ foreach(l, elemlist)
+ {
+ /* Note that filename was already canonicalized */
+ char *filename = (char *) lfirst(l);
+ char *expanded = NULL;
+
+ /* If restricting, insert $libdir/plugins if not mentioned already */
+ if (restricted && first_dir_separator(filename) == NULL)
+ {
+ expanded = psprintf("$libdir/plugins/%s", filename);
+ filename = expanded;
+ }
+
+ /*
+ * stat()/access() only check that the library exists, not that it has
+ * the correct magic number or even a library. But error messages from
+ * dlopen() are not portable, so it'd be hard to report any problem
+ * other than "does not exist".
+ */
+ if (access(filename, R_OK) == 0) // F_OK
+ continue;
+
+ if (is_session)
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errhint("New sessions will fail with the existing configuration."));
+ else
+ ereport(WARNING,
+ errcode_for_file_access(),
+ errmsg("could not access file \"%s\"", filename),
+ errhint("The server will fail to start with the existing configuration."
+ " If it is is shut down, it will be necessary to manually edit the %s file to allow it to start.",
+ "postgresql.auto.conf"));
+ }
+
+ list_free_deep(elemlist);
+ pfree(rawstring);
+ return true;
+}
+
+static bool
+check_shared_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true, false);
+}
+
+static bool
+check_local_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, false, false);
+}
+
+static bool
+check_session_preload_libraries(char **newval, void **extra, GucSource source)
+{
+ return check_preload_libraries(newval, extra, source, true, true);
+}
+
static bool
check_effective_io_concurrency(int *newval, void **extra, GucSource source)
{
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index eb608fdc2ea..368b10558d7 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1066,6 +1066,7 @@ GRANT pg_read_all_settings TO regress_role_haspriv;
BEGIN;
-- A GUC using GUC_SUPERUSER_ONLY is useful for negative tests.
SET LOCAL session_preload_libraries TO 'path-to-preload-libraries';
+WARNING: could not access file "$libdir/plugins/path-to-preload-libraries"
SET SESSION AUTHORIZATION regress_role_haspriv;
-- passes with role member of pg_read_all_settings
SHOW session_preload_libraries;
diff --git a/src/test/modules/worker_spi/expected/worker_spi.out b/src/test/modules/worker_spi/expected/worker_spi.out
index dc0a79bf759..5b275669bcf 100644
--- a/src/test/modules/worker_spi/expected/worker_spi.out
+++ b/src/test/modules/worker_spi/expected/worker_spi.out
@@ -28,6 +28,8 @@ SELECT pg_reload_conf();
t
(1 row)
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/modules/worker_spi/sql/worker_spi.sql b/src/test/modules/worker_spi/sql/worker_spi.sql
index 4683523b29d..4ed5370d456 100644
--- a/src/test/modules/worker_spi/sql/worker_spi.sql
+++ b/src/test/modules/worker_spi/sql/worker_spi.sql
@@ -18,6 +18,8 @@ END
$$;
INSERT INTO schema4.counted VALUES ('total', 0), ('delta', 1);
SELECT pg_reload_conf();
+-- reconnect to avoid unstable test result due to asynchronous signal
+\c
-- wait until the worker has processed the tuple we just inserted
DO $$
DECLARE
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 1420288d67b..f3d33dda95e 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3362,6 +3362,7 @@ drop table hats;
drop table hat_data;
-- test for pg_get_functiondef properly regurgitating SET parameters
-- Note that the function is kept around to stress pg_dump.
+SET check_function_bodies=no;
CREATE FUNCTION func_with_set_params() RETURNS integer
AS 'select 1;'
LANGUAGE SQL
@@ -3371,6 +3372,14 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
SET datestyle to iso, mdy
SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
IMMUTABLE STRICT;
+WARNING: could not access file "Mixed/Case"
+HINT: The server will fail to start with the existing configuration. If it is is shut down, it will be necessary to manually edit the postgresql.auto.conf file to allow it to start.
+WARNING: could not access file "c:/'a"/path"
+HINT: The server will fail to start with the existing configuration. If it is is shut down, it will be necessary to manually edit the postgresql.auto.conf file to allow it to start.
+WARNING: could not access file ""
+HINT: The server will fail to start with the existing configuration. If it is is shut down, it will be necessary to manually edit the postgresql.auto.conf file to allow it to start.
+WARNING: could not access file "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
+HINT: The server will fail to start with the existing configuration. If it is is shut down, it will be necessary to manually edit the postgresql.auto.conf file to allow it to start.
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
pg_get_functiondef
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 8bdab6dec30..6308e5d27fd 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1198,6 +1198,7 @@ drop table hat_data;
-- test for pg_get_functiondef properly regurgitating SET parameters
-- Note that the function is kept around to stress pg_dump.
+SET check_function_bodies=no;
CREATE FUNCTION func_with_set_params() RETURNS integer
AS 'select 1;'
LANGUAGE SQL
--
2.17.1
--1Ow488MNN9B9o/ov
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0002-errcontext-if-server-fails-to-start-due-to-librar.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: On disable_cost
@ 2024-09-06 09:00 Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Lakhin @ 2024-09-06 09:00 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: David Rowley <[email protected]>; Heikki Linnakangas <[email protected]>; Peter Geoghegan <[email protected]>; PostgreSQL Hackers <[email protected]>
Hello Robert,
21.08.2024 17:29, Robert Haas wrote:
> I went ahead and committed these patches. ...
Please take a look at the following code:
static void
label_sort_with_costsize(PlannerInfo *root, Sort *plan, double limit_tuples)
{
...
cost_sort(&sort_path, root, NIL,
lefttree->total_cost,
plan->plan.disabled_nodes,
lefttree->plan_rows,
lefttree->plan_width,
0.0,
work_mem,
limit_tuples);
Given the cost_sort() declaration:
void
cost_sort(Path *path, PlannerInfo *root,
List *pathkeys, int input_disabled_nodes,
Cost input_cost, double tuples, int width,
Cost comparison_cost, int sort_mem,
double limit_tuples)
Aren't the input_disabled_nodes and input_cost arguments swapped in the
above call?
(I've discovered this with UBSan, which complained
createplan.c:5457:6: runtime error: 4.40465e+09 is outside the range of representable values of type 'int'
while executing a query with a large estimated cost.)
Best regards,
Alexander
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: On disable_cost
@ 2024-09-06 09:27 Richard Guo <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guo @ 2024-09-06 09:27 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Robert Haas <[email protected]>; Tom Lane <[email protected]>; David Rowley <[email protected]>; Heikki Linnakangas <[email protected]>; Peter Geoghegan <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Sep 6, 2024 at 5:00 PM Alexander Lakhin <[email protected]> wrote:
> static void
> label_sort_with_costsize(PlannerInfo *root, Sort *plan, double limit_tuples)
> {
> ...
> cost_sort(&sort_path, root, NIL,
> lefttree->total_cost,
> plan->plan.disabled_nodes,
> lefttree->plan_rows,
> lefttree->plan_width,
> 0.0,
> work_mem,
> limit_tuples);
>
> Given the cost_sort() declaration:
> void
> cost_sort(Path *path, PlannerInfo *root,
> List *pathkeys, int input_disabled_nodes,
> Cost input_cost, double tuples, int width,
> Cost comparison_cost, int sort_mem,
> double limit_tuples)
>
> Aren't the input_disabled_nodes and input_cost arguments swapped in the
> above call?
Nice catch! I checked other callers to cost_sort, and they are all
good.
(I'm a little surprised that this does not cause any plan diffs in the
regression tests.)
Thanks
Richard
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: On disable_cost
@ 2024-09-09 04:09 Richard Guo <[email protected]>
parent: Richard Guo <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guo @ 2024-09-09 04:09 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Robert Haas <[email protected]>; Tom Lane <[email protected]>; David Rowley <[email protected]>; Heikki Linnakangas <[email protected]>; Peter Geoghegan <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Sep 6, 2024 at 5:27 PM Richard Guo <[email protected]> wrote:
> On Fri, Sep 6, 2024 at 5:00 PM Alexander Lakhin <[email protected]> wrote:
> > static void
> > label_sort_with_costsize(PlannerInfo *root, Sort *plan, double limit_tuples)
> > {
> > ...
> > cost_sort(&sort_path, root, NIL,
> > lefttree->total_cost,
> > plan->plan.disabled_nodes,
> > lefttree->plan_rows,
> > lefttree->plan_width,
> > 0.0,
> > work_mem,
> > limit_tuples);
> >
> > Given the cost_sort() declaration:
> > void
> > cost_sort(Path *path, PlannerInfo *root,
> > List *pathkeys, int input_disabled_nodes,
> > Cost input_cost, double tuples, int width,
> > Cost comparison_cost, int sort_mem,
> > double limit_tuples)
> >
> > Aren't the input_disabled_nodes and input_cost arguments swapped in the
> > above call?
>
> Nice catch! I checked other callers to cost_sort, and they are all
> good.
Fixed.
Thanks
Richard
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: On disable_cost
@ 2024-09-09 15:28 Robert Haas <[email protected]>
parent: Richard Guo <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Robert Haas @ 2024-09-09 15:28 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; David Rowley <[email protected]>; Heikki Linnakangas <[email protected]>; Peter Geoghegan <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Sep 9, 2024 at 12:09 AM Richard Guo <[email protected]> wrote:
> Fixed.
Thanks to Alexander for the very good catch and to Richard for pushing the fix.
(I started to respond to this last week but didn't quite get to it
before I ran out of time/energy.)
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v1 1/1] Use a non-locking initial test in TAS_SPIN on AArch64.
@ 2024-10-22 19:33 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Nathan Bossart @ 2024-10-22 19:33 UTC (permalink / raw)
---
src/include/storage/s_lock.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index e94ed5f48b..07f343baf8 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -270,6 +270,12 @@ tas(volatile slock_t *lock)
*/
#if defined(__aarch64__)
+/*
+ * On ARM64, it's a win to use a non-locking test before the TAS proper. It
+ * may be a win on 32-bit ARM, too, but nobody's tested it yet.
+ */
+#define TAS_SPIN(lock) (*(lock) ? 1 : TAS(lock))
+
#define SPIN_DELAY() spin_delay()
static __inline__ void
--
2.39.5 (Apple Git-154)
--cHDCaEHhDdnHUGIr--
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v1 1/1] Use a non-locking initial test in TAS_SPIN on AArch64.
@ 2024-10-22 19:33 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Nathan Bossart @ 2024-10-22 19:33 UTC (permalink / raw)
---
src/include/storage/s_lock.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index e94ed5f48b..07f343baf8 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -270,6 +270,12 @@ tas(volatile slock_t *lock)
*/
#if defined(__aarch64__)
+/*
+ * On ARM64, it's a win to use a non-locking test before the TAS proper. It
+ * may be a win on 32-bit ARM, too, but nobody's tested it yet.
+ */
+#define TAS_SPIN(lock) (*(lock) ? 1 : TAS(lock))
+
#define SPIN_DELAY() spin_delay()
static __inline__ void
--
2.39.5 (Apple Git-154)
--cHDCaEHhDdnHUGIr--
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2024-10-22 19:33 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 14:42 [PATCH v3 2/2] warn when setting GUC to a nonextant library Justin Pryzby <[email protected]>
2021-12-13 14:42 [PATCH 2/4] warn when setting GUC to a nonextant library Justin Pryzby <[email protected]>
2021-12-13 14:42 [PATCH v5 1/3] warn when setting GUC to a nonextant library Justin Pryzby <[email protected]>
2021-12-13 14:42 [PATCH v4 1/3] warn when setting GUC to a nonextant library Justin Pryzby <[email protected]>
2021-12-13 14:42 [PATCH v6 2/4] warn when setting GUC to a nonextant library Justin Pryzby <[email protected]>
2024-09-06 09:00 Re: On disable_cost Alexander Lakhin <[email protected]>
2024-09-06 09:27 ` Re: On disable_cost Richard Guo <[email protected]>
2024-09-09 04:09 ` Re: On disable_cost Richard Guo <[email protected]>
2024-09-09 15:28 ` Re: On disable_cost Robert Haas <[email protected]>
2024-10-22 19:33 [PATCH v1 1/1] Use a non-locking initial test in TAS_SPIN on AArch64. Nathan Bossart <[email protected]>
2024-10-22 19:33 [PATCH v1 1/1] Use a non-locking initial test in TAS_SPIN on AArch64. Nathan Bossart <[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