From f74a024c33702d46af1261ac4c6314576018b4fe Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 3 Sep 2013 18:32:35 -0400 Subject: [PATCH] Don't fail for bad GUCs in CREATE FUNCTION with check_function_bodies off. The previous coding attempted to activate all the GUC settings specified in SET clauses, so that the function validator could operate in the GUC environment expected by the function body. However, this is problematic when restoring a dump, since the SET clauses might refer to database objects that don't exist yet. We already have the parameter check_function_bodies that's meant to prevent forward references in function definitions from breaking dumps, so let's change CREATE FUNCTION to not install the SET values if check_function_bodies is off. Authors of function validators were already advised not to make any "context sensitive" checks when check_function_bodies is off, if indeed they're checking anything at all in that mode. But extend the documentation to point out the GUC issue in particular. (Note that we still check the SET clauses to some extent; the behavior with !check_function_bodies is now approximately equivalent to what ALTER DATABASE/ROLE have been doing for awhile with context-dependent GUCs.) This problem can be demonstrated in all active branches, so back-patch all the way. --- src/backend/catalog/pg_proc.c | 34 ++++++++++++++++++++----------- src/test/regress/expected/guc.out | 16 +++++++++++++++ src/test/regress/sql/guc.sql | 18 ++++++++++++++++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index f84bfe4f6ab..3a2c817a8b3 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -561,24 +561,34 @@ ProcedureCreate(const char *procedureName, /* Verify function body */ if (OidIsValid(languageValidator)) { - ArrayType *set_items; - int save_nestlevel; + ArrayType *set_items = NULL; + int save_nestlevel = 0; /* Advance command counter so new tuple can be seen by validator */ CommandCounterIncrement(); - /* Set per-function configuration parameters */ - set_items = (ArrayType *) DatumGetPointer(proconfig); - if (set_items) /* Need a new GUC nesting level */ + /* + * Set per-function configuration parameters so that the validation is + * done with the environment the function expects. However, if + * check_function_bodies is off, we don't do this, because that would + * create dump ordering hazards that pg_dump doesn't know how to deal + * with. (For example, a SET clause might refer to a not-yet-created + * text search configuration.) This means that the validator + * shouldn't complain about anything that might depend on a GUC + * parameter when check_function_bodies is off. + */ + if (check_function_bodies) { - save_nestlevel = NewGUCNestLevel(); - ProcessGUCArray(set_items, - (superuser() ? PGC_SUSET : PGC_USERSET), - PGC_S_SESSION, - GUC_ACTION_SAVE); + set_items = (ArrayType *) DatumGetPointer(proconfig); + if (set_items) /* Need a new GUC nesting level */ + { + save_nestlevel = NewGUCNestLevel(); + ProcessGUCArray(set_items, + (superuser() ? PGC_SUSET : PGC_USERSET), + PGC_S_SESSION, + GUC_ACTION_SAVE); + } } - else - save_nestlevel = 0; /* keep compiler quiet */ OidFunctionCall1(languageValidator, ObjectIdGetDatum(retval)); diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index a580daa089d..0d1065f66fa 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -699,3 +699,19 @@ select myfunc(1), current_setting('regex_flavor'); extended | extended (1 row) +-- Normally, CREATE FUNCTION should complain about invalid values in +-- function SET options; but not if check_function_bodies is off, +-- because that creates ordering hazards for pg_dump +create function func_with_bad_set() returns int as $$ select 1 $$ +language sql +set default_text_search_config = no_such_config; +NOTICE: text search configuration "no_such_config" does not exist +ERROR: invalid value for parameter "default_text_search_config": "no_such_config" +set check_function_bodies = off; +create function func_with_bad_set() returns int as $$ select 1 $$ +language sql +set default_text_search_config = no_such_config; +NOTICE: text search configuration "no_such_config" does not exist +select func_with_bad_set(); +ERROR: invalid value for parameter "default_text_search_config": "no_such_config" +reset check_function_bodies; diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index 9966606275c..e3c1b33c0fe 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -251,3 +251,21 @@ set regex_flavor = basic; select myfunc(0); select current_setting('regex_flavor'); select myfunc(1), current_setting('regex_flavor'); + +-- Normally, CREATE FUNCTION should complain about invalid values in +-- function SET options; but not if check_function_bodies is off, +-- because that creates ordering hazards for pg_dump + +create function func_with_bad_set() returns int as $$ select 1 $$ +language sql +set default_text_search_config = no_such_config; + +set check_function_bodies = off; + +create function func_with_bad_set() returns int as $$ select 1 $$ +language sql +set default_text_search_config = no_such_config; + +select func_with_bad_set(); + +reset check_function_bodies;