"SET search_path" clause ignored during function creation
As discussed in irc://freenode/postgresql (2010-05-06 16:20)
Function bodies are checked using the _current_ search_path instead of
the search_path supplied by the "SET search_path" clause.
This leads to wrong error messages during creation.
On the other hand it can make a function body pass the check, even
though it will raise an error is use.
Proposed solution: Function bodies should be checked with the
search_path provided by "SET search_path" an _not_ with the current
search path at the time pof creation.
Ho to reproduce the bug:
/*
event=# show search_path;
search_path
-------------
public
*/
CREATE SCHEMA foo;
CREATE TABLE foo.adr
( adr_id integer primary key,
note text);
INSERT INTO foo.adr VALUES (1, 'note from table foo.adr');
CREATE FUNCTION f_test()
RETURNS text AS
'SELECT note FROM adr where adr_id = 1'
LANGUAGE 'sql'
SET search_path=foo;
-- ERROR: relation "adr" does not exist
-- LINE 3: 'SELECT note FROM adr where adr_id = 1'
-- Function body is falsely checked with "search_path=
public" (current search_path) instead of "search_path=foo" AS it
should be!
-- If I disable check_function_bodies before creation, creation works
and the function call works and returns the value of foo.adr
correctly.
SET check_function_bodies=false;
Regards
Erwin Brandstetter
Erwin Brandstetter <brsaweda@gmail.com> wrote:
Function bodies are checked using the _current_ search_path instead of
the search_path supplied by the "SET search_path" clause.Proposed solution: Function bodies should be checked with the
search_path provided by "SET search_path" an _not_ with the current
search path at the time pof creation.
Thanks for the report! Please check whether the attached patch
is the correct fix. An additional regression test is included.
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
Attachments:
create_function_with_search_path.patchapplication/octet-stream; name=create_function_with_search_path.patchDownload
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index d76e415..04f136d 100644
*** a/src/backend/catalog/pg_proc.c
--- b/src/backend/catalog/pg_proc.c
*************** ProcedureCreate(const char *procedureNam
*** 621,629 ****
--- 621,647 ----
/* Verify function body */
if (OidIsValid(languageValidator))
{
+ ArrayType *set_items;
+ int save_nestlevel;
+
/* 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 */
+ {
+ save_nestlevel = NewGUCNestLevel();
+ ProcessGUCArray(set_items,
+ (superuser() ? PGC_SUSET : PGC_USERSET),
+ PGC_S_SESSION,
+ GUC_ACTION_SAVE);
+ }
+
OidFunctionCall1(languageValidator, ObjectIdGetDatum(retval));
+
+ if (set_items)
+ AtEOXact_GUC(true, save_nestlevel);
}
return retval;
diff --git a/src/test/regress/input/create_function_2.source b/src/test/regress/input/create_function_2.source
index b1289e8..9d2c0d7 100644
*** a/src/test/regress/input/create_function_2.source
--- b/src/test/regress/input/create_function_2.source
*************** CREATE FUNCTION oldstyle_length(int4, te
*** 64,69 ****
--- 64,80 ----
AS '@libdir@/regress@DLSUFFIX@'
LANGUAGE C;
+ CREATE SCHEMA non_default_search_path;
+ CREATE TABLE non_default_search_path.table_in_custom_search_path (id integer);
+ CREATE FUNCTION function_with_search_path()
+ RETURNS SETOF integer
+ AS 'SELECT * FROM table_in_custom_search_path'
+ LANGUAGE SQL
+ SET search_path = non_default_search_path;
+ SELECT * FROM function_with_search_path();
+ DROP TABLE non_default_search_path.table_in_custom_search_path;
+ DROP SCHEMA non_default_search_path;
+
--
-- Function dynamic loading
--
diff --git a/src/test/regress/output/create_function_2.source b/src/test/regress/output/create_function_2.source
index 0feb975..41ab7d7 100644
*** a/src/test/regress/output/create_function_2.source
--- b/src/test/regress/output/create_function_2.source
*************** CREATE FUNCTION oldstyle_length(int4, te
*** 51,56 ****
--- 51,70 ----
RETURNS int4
AS '@libdir@/regress@DLSUFFIX@'
LANGUAGE C;
+ CREATE SCHEMA non_default_search_path;
+ CREATE TABLE non_default_search_path.table_in_custom_search_path (id integer);
+ CREATE FUNCTION function_with_search_path()
+ RETURNS SETOF integer
+ AS 'SELECT * FROM table_in_custom_search_path'
+ LANGUAGE SQL
+ SET search_path = non_default_search_path;
+ SELECT * FROM function_with_search_path();
+ function_with_search_path
+ ---------------------------
+ (0 rows)
+
+ DROP TABLE non_default_search_path.table_in_custom_search_path;
+ DROP SCHEMA non_default_search_path;
--
-- Function dynamic loading
--
Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> writes:
Thanks for the report! Please check whether the attached patch
is the correct fix. An additional regression test is included.
That's going to provoke "uninitialized variable" compiler warnings,
but otherwise it seems reasonably sane.
I don't particularly see the point of the added regression test.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote:
Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> writes:
Thanks for the report! Please check whether the attached patch
is the correct fix. An additional regression test is included.That's going to provoke "uninitialized variable" compiler warnings,
but otherwise it seems reasonably sane.
I applied a revised version that can surpress compiler warnings
to 9.0beta, 8.4 and 8.3.
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center