From 880bfc3287dd68cfe90d10d9597d7b0fd2dae3e5 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 11 Apr 2012 11:29:22 -0400 Subject: [PATCH] Silently ignore any nonexistent schemas that are listed in search_path. Previously we attempted to throw an error or at least warning for missing schemas, but this was done inconsistently because of implementation restrictions (in many cases, GUC settings are applied outside transactions so that we can't do system catalog lookups). Furthermore, there were exceptions to the rule even in the beginning, and we'd been poking more and more holes in it as time went on, because it turns out that there are lots of use-cases for having some irrelevant items in a common search_path value. It seems better to just adopt a philosophy similar to what's always been done with Unix PATH settings, wherein nonexistent or unreadable directories are silently ignored. This commit also fixes the documentation to point out that schemas for which the user lacks USAGE privilege are silently ignored. That's always been true but was previously not documented. This is mostly in response to Robert Haas' complaint that 9.1 started to throw errors or warnings for missing schemas in cases where prior releases had not. We won't adopt such a significant behavioral change in a back branch, so something different will be needed in 9.1. --- doc/src/sgml/config.sgml | 26 ++++++++++------ src/backend/catalog/namespace.c | 51 +++++-------------------------- src/test/regress/expected/guc.out | 34 ++++++++++++++++----- src/test/regress/sql/guc.sql | 20 +++++++----- 4 files changed, 62 insertions(+), 69 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 640defde86..361ad7b99a 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4670,10 +4670,17 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; The value for search_path must be a comma-separated - list of schema names. If one of the list items is - the special value $user, then the schema - having the name returned by SESSION_USER is substituted, if there - is such a schema. (If not, $user is ignored.) + list of schema names. Any name that is not an existing schema, or is + a schema for which the user does not have USAGE + permission, is silently ignored. + + + + If one of the list items is the special name + $user, then the schema having the name returned by + SESSION_USER is substituted, if there is such a schema + and the user has USAGE permission for it. + (If not, $user is ignored.) @@ -4697,16 +4704,15 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; When objects are created without specifying a particular target - schema, they will be placed in the first schema listed - in the search path. An error is reported if the search path is - empty. + schema, they will be placed in the first valid schema named in + search_path. An error is reported if the search + path is empty. The default value for this parameter is - '"$user", public' (where the second part will be - ignored if there is no schema named public). - This supports shared use of a database (where no users + "$user", public. + This setting supports shared use of a database (where no users have private schemas, and all share use of public), private per-user schemas, and combinations of these. Other effects can be obtained by altering the default search path diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index dc8f8eaf3f..e92efd863e 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -3773,14 +3773,12 @@ ResetTempTableNamespace(void) * Routines for handling the GUC variable 'search_path'. */ -/* check_hook: validate new search_path, if possible */ +/* check_hook: validate new search_path value */ bool check_search_path(char **newval, void **extra, GucSource source) { - bool result = true; char *rawname; List *namelist; - ListCell *l; /* Need a modifiable copy of string */ rawname = pstrdup(*newval); @@ -3796,52 +3794,17 @@ check_search_path(char **newval, void **extra, GucSource source) } /* - * If we aren't inside a transaction, we cannot do database access so - * cannot verify the individual names. Must accept the list on faith. + * We used to try to check that the named schemas exist, but there are + * many valid use-cases for having search_path settings that include + * schemas that don't exist; and often, we are not inside a transaction + * here and so can't consult the system catalogs anyway. So now, the only + * requirement is syntactic validity of the identifier list. */ - if (IsTransactionState()) - { - /* - * Verify that all the names are either valid namespace names or - * "$user" or "pg_temp". We do not require $user to correspond to a - * valid namespace, and pg_temp might not exist yet. We do not check - * for USAGE rights, either; should we? - * - * When source == PGC_S_TEST, we are checking the argument of an ALTER - * DATABASE SET or ALTER USER SET command. It could be that the - * intended use of the search path is for some other database, so we - * should not error out if it mentions schemas not present in the - * current database. We issue a NOTICE instead. - */ - foreach(l, namelist) - { - char *curname = (char *) lfirst(l); - - if (strcmp(curname, "$user") == 0) - continue; - if (strcmp(curname, "pg_temp") == 0) - continue; - if (!SearchSysCacheExists1(NAMESPACENAME, - CStringGetDatum(curname))) - { - if (source == PGC_S_TEST) - ereport(NOTICE, - (errcode(ERRCODE_UNDEFINED_SCHEMA), - errmsg("schema \"%s\" does not exist", curname))); - else - { - GUC_check_errdetail("schema \"%s\" does not exist", curname); - result = false; - break; - } - } - } - } pfree(rawname); list_free(namelist); - return result; + return true; } /* assign_hook: do extra actions as needed */ diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index d324862049..271706d31e 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -605,6 +605,31 @@ SELECT current_user = 'temp_reset_user'; DROP ROLE temp_reset_user; -- +-- search_path should react to changes in pg_namespace +-- +set search_path = foo, public, not_there_initially; +select current_schemas(false); + current_schemas +----------------- + {public} +(1 row) + +create schema not_there_initially; +select current_schemas(false); + current_schemas +------------------------------ + {public,not_there_initially} +(1 row) + +drop schema not_there_initially; +select current_schemas(false); + current_schemas +----------------- + {public} +(1 row) + +reset search_path; +-- -- Tests for function-local GUC settings -- set work_mem = '3MB'; @@ -617,14 +642,7 @@ select report_guc('work_mem'), current_setting('work_mem'); 1MB | 3MB (1 row) --- this should draw only a warning -alter function report_guc(text) set search_path = no_such_schema; -NOTICE: schema "no_such_schema" does not exist --- with error occurring here -select report_guc('work_mem'), current_setting('work_mem'); -ERROR: invalid value for parameter "search_path": "no_such_schema" -DETAIL: schema "no_such_schema" does not exist -alter function report_guc(text) reset search_path set work_mem = '2MB'; +alter function report_guc(text) set work_mem = '2MB'; select report_guc('work_mem'), current_setting('work_mem'); report_guc | current_setting ------------+----------------- diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index 21ed86f26b..0c21792381 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -182,6 +182,18 @@ SELECT relname from pg_class where relname = 'tmp_foo'; SELECT current_user = 'temp_reset_user'; DROP ROLE temp_reset_user; +-- +-- search_path should react to changes in pg_namespace +-- + +set search_path = foo, public, not_there_initially; +select current_schemas(false); +create schema not_there_initially; +select current_schemas(false); +drop schema not_there_initially; +select current_schemas(false); +reset search_path; + -- -- Tests for function-local GUC settings -- @@ -194,13 +206,7 @@ set work_mem = '1MB'; select report_guc('work_mem'), current_setting('work_mem'); --- this should draw only a warning -alter function report_guc(text) set search_path = no_such_schema; - --- with error occurring here -select report_guc('work_mem'), current_setting('work_mem'); - -alter function report_guc(text) reset search_path set work_mem = '2MB'; +alter function report_guc(text) set work_mem = '2MB'; select report_guc('work_mem'), current_setting('work_mem');