public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v6 2/4] warn when setting GUC to a nonextant library 3+ messages / 3 participants [nested] [flat]
* [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; 3+ 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] 3+ messages in thread
* [PATCH] Add tests for psql tab completion @ 2022-02-12 20:17 Matheus Alcantara <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Matheus Alcantara @ 2022-02-12 20:17 UTC (permalink / raw) To: pgsql-hackers Hi hackers. I'm attaching a patch that add some new test cases for tab completion of psql. This is my first patch that I'm sending here so let me know if I'm doing something wrong. -- Matheus Alcantara Attachments: [text/x-patch] 0001-psql-Add-tests-for-tab-completion.patch (3.0K, ../../cyYnAHiGk0M96LLOz3UUdsndyxD0LFdzDSJmCT49bKlPkr-p4GassX9_hEzWup_kkFr9n6ETce7BwvVRoxL59iJlkCNk1mPL82W2rXVeGX4=@pm.me/2-0001-psql-Add-tests-for-tab-completion.patch) download | inline diff: From 6af6b972960f4e9017f1c311ee4b13a96d5f66a1 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Sat, 12 Feb 2022 16:45:55 -0300 Subject: [PATCH] psql: Add tests for tab completion --- src/bin/psql/t/010_tab_completion.pl | 92 ++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl index 005961f34d..f74ff96226 100644 --- a/src/bin/psql/t/010_tab_completion.pl +++ b/src/bin/psql/t/010_tab_completion.pl @@ -45,6 +45,7 @@ $node->safe_psql('postgres', . "CREATE TABLE mytab246 (f1 int, f2 text);\n" . "CREATE TABLE \"mixedName\" (f1 int, f2 text);\n" . "CREATE TYPE enum1 AS ENUM ('foo', 'bar', 'baz', 'BLACK');\n" + . "CREATE INDEX idx_tab1_c1 ON tab1(c1);\n" . "CREATE PUBLICATION some_publication;\n"); # Developers would not appreciate this test adding a bunch of junk to @@ -382,6 +383,97 @@ check_completion( clear_query(); +check_completion( + "CREATE OR REPLACE \t\t", + qr/AGGREGATE +LANGUAGE +RULE +TRIGGER +\r\nFUNCTION +PROCEDURE +TRANSFORM +VIEW/, + "check create or replace"); + +clear_query(); + +check_completion( + "ALTER FOREIGN \t\t", + qr/DATA WRAPPER +TABLE/ , + "check alter foreign"); + +clear_query(); + +check_completion( + "ALTER INDEX \t\t", + qr/ALL IN TABLESPACE +information_schema. +tab1_pkey\r\nidx_tab1_c1/, + "check alter index"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 \t\t", + qr/ALTER COLUMN +NO DEPENDS ON EXTENSION +RESET\r\nATTACH PARTITION +OWNER TO +SET\r\nDEPENDS ON EXTENSION +RENAME TO/, + "check alter index <name>"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 ATTACH \t\t", + qr/PARTITION/, + "check alter index <name> attach"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 ATTACH PARTITION \t\t", + qr/idx_tab1_c1 +public. +\r\ninformation_schema. +tab1_pkey/, + "check alter index <name> ATTACH PARTITION"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 ALTER \t\t", + qr/COLUMN/, + "check alter index <name> alter"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 ALTER COLUMN \t\t", + qr/1 +SET +STATISTICS/, + "check alter index <name> alter column"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 ALTER COLUMN 1 SET \t\t", + qr/STATISTICS/, + "check alter index <name> alter column <column> set"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 SET \t\t", + qr/\( +TABLESPACE/, + "check alter index <name> set"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 RESET \t\t", + qr/\( +\a/, + "check alter index <name> reset"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 NO DEPENDS \t\t", + qr/ON EXTENSION/, + "check alter index <name> no depends"); + +clear_query(); + +check_completion( + "ALTER INDEX idx_tab1_c1 DEPENDS \t\t", + qr/ON EXTENSION/, + "check alter index <name> depends"); + +clear_query(); + # check VersionedQuery infrastructure check_completion( "DROP PUBLIC\t \t\t", -- 2.35.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Add tests for psql tab completion @ 2022-02-14 20:01 Tom Lane <[email protected]> parent: Matheus Alcantara <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2022-02-14 20:01 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: pgsql-hackers Matheus Alcantara <[email protected]> writes: > I'm attaching a patch that add some new test cases for tab completion of psql. What exactly is the motivation for these particular tests? I believe that most of tab-complete.c is already covered, outside of the giant if-else chain at the heart of psql_completion(). It's hard to summon interest in trying to cover every branch of that chain; but if we're to add coverage of just a few more, which ones and why? The ones you've chosen to hit in this patch don't seem especially interesting. regards, tom lane ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-02-14 20:01 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-12-13 14:42 [PATCH v6 2/4] warn when setting GUC to a nonextant library Justin Pryzby <[email protected]> 2022-02-12 20:17 [PATCH] Add tests for psql tab completion Matheus Alcantara <[email protected]> 2022-02-14 20:01 ` Re: [PATCH] Add tests for psql tab completion Tom Lane <[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