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.

diff --git a/doc/src/sgml/plhandler.sgml b/doc/src/sgml/plhandler.sgml
index 20021bce6504f9feaccd103a99e2d9a848a5836f..c9b804c0752180ec4ee658a007fe46f6e3ed2a72 100644 (file)
--- a/doc/src/sgml/plhandler.sgml
+++ b/doc/src/sgml/plhandler.sgml
@@ -201,6 +201,13 @@ CREATE LANGUAGE plsample
     of having a validator is not to let the call handler omit checks, but
     to notify the user immediately if there are obvious errors in a
     <command>CREATE FUNCTION</> command.)
+    While the choice of exactly what to check is mostly left to the
+    discretion of the validator function, note that the core
+    <command>CREATE FUNCTION</> code only executes <literal>SET</> clauses
+    attached to a function when <varname>check_function_bodies</> is on.
+    Therefore, checks whose results might be affected by GUC parameters
+    definitely should be skipped when <varname>check_function_bodies</> is
+    off, to avoid false failures when reloading a dump.
    </para>
 
    <para>

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 72559daf54975eb8e706aaf0f8ee5de9fa4b318f..ca21faf7b436780d39ca32a773253a72b238fdff 100644 (file)
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -633,24 +633,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 98671a2bcdf892cd0573fd1e8ed7e2d0bcf2d3af..ef0d807011a1edda9e3273d2be408a7cd1e9898a 100644 (file)
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -700,3 +700,19 @@ select myfunc(1), current_setting('work_mem');
  2MB    | 2MB
 (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 21ed86f26ba259e7258e6b69afa8f8e80e2a2442..d7c57a560b878af8faa7fabe25fa25cb34a8f6ed 100644 (file)
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -251,3 +251,21 @@ set work_mem = '1MB';
 select myfunc(0);
 select current_setting('work_mem');
 select myfunc(1), current_setting('work_mem');
+
+-- 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;