Thread: pg_dump does not honor namespaces when functions are used in index

Dear hackers,

I have a pretty nasty problem to submit to your careful scrutiny.

Please consider the following piece of SQL code:


CREATE SCHEMA bar;
SET search_path = bar;

CREATE FUNCTION bar() RETURNS text AS $$
BEGIN   RETURN 'foobar';
END
$$ LANGUAGE plpgsql IMMUTABLE;

CREATE SCHEMA foo;
SET search_path = foo;

CREATE FUNCTION foo() RETURNS text AS $$
BEGIN   RETURN bar();
END
$$ LANGUAGE plpgsql IMMUTABLE;

SET search_path = public;

CREATE TABLE foobar (d text);
insert into foobar (d) values ('foobar');

set search_path = public, foo, bar;
CREATE INDEX foobar_d on foobar using btree(foo());


Run this on a newly created database, and dump it with pg_dump. You'll
notice that the dump is unusable.  Creating a new database from this
dump will trigger the following error:

ERROR:  function bar() does not exist
LINE 1: SELECT bar()              ^
HINT:  No function matches the given name and argument types. You
might need to add explicit type casts.
QUERY:  SELECT bar()
CONTEXT:  PL/pgSQL function "foo" line 2 at RETURN

How can we fix this?
-- 
Jean-Baptiste Quenot

<div class="gmail_quote">On 17 June 2010 12:31, Jean-Baptiste Quenot <span dir="ltr"><<a
href="mailto:jbq@caraldi.com">jbq@caraldi.com</a>></span>wrote:<br /><blockquote class="gmail_quote" style="margin:
0pt0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> Dear hackers,<br /><br /> I have a
prettynasty problem to submit to your careful scrutiny.<br /><br /> Please consider the following piece of SQL code:<br
/><br/><br /> CREATE SCHEMA bar;<br /> SET search_path = bar;<br /><br /> CREATE FUNCTION bar() RETURNS text AS $$<br
/>BEGIN<br />    RETURN 'foobar';<br /> END<br /> $$ LANGUAGE plpgsql IMMUTABLE;<br /><br /> CREATE SCHEMA foo;<br />
SETsearch_path = foo;<br /><br /> CREATE FUNCTION foo() RETURNS text AS $$<br /> BEGIN<br />    RETURN bar();<br />
END<br/> $$ LANGUAGE plpgsql IMMUTABLE;<br /><br /> SET search_path = public;<br /><br /> CREATE TABLE foobar (d
text);<br/> insert into foobar (d) values ('foobar');<br /><br /> set search_path = public, foo, bar;<br /> CREATE
INDEXfoobar_d on foobar using btree(foo());<br /><br /><br /> Run this on a newly created database, and dump it with
pg_dump.You'll<br /> notice that the dump is unusable.  Creating a new database from this<br /> dump will trigger the
followingerror:<br /><br /> ERROR:  function bar() does not exist<br /> LINE 1: SELECT bar()<br />               ^<br
/>HINT:  No function matches the given name and argument types. You<br /> might need to add explicit type casts.<br />
QUERY: SELECT bar()<br /> CONTEXT:  PL/pgSQL function "foo" line 2 at RETURN<br /><br /> How can we fix this?<br />
--<br/> Jean-Baptiste Quenot<br /><font color="#888888"><br /> --</font><br /></blockquote></div><br />I think Postgres
doesn'tcheck to see whether bar() exists in the current search path when you create the foo() function, and since it
isn'tin the foo() function's search path value, it fails to find the function when you try to use it.  It can probably
befixed (this specific case, not generally) with:<br /><br />ALTER FUNCTION foo.foo() SET search_path=foo, bar;<br
/><br/>Thom<br /> 

On Thu, Jun 17, 2010 at 8:13 AM, Thom Brown <thombrown@gmail.com> wrote:
> On 17 June 2010 12:31, Jean-Baptiste Quenot <jbq@caraldi.com> wrote:
>>
>> Dear hackers,
>>
>> I have a pretty nasty problem to submit to your careful scrutiny.
>>
>> Please consider the following piece of SQL code:
>>
>>
>> CREATE SCHEMA bar;
>> SET search_path = bar;
>>
>> CREATE FUNCTION bar() RETURNS text AS $$
>> BEGIN
>>    RETURN 'foobar';
>> END
>> $$ LANGUAGE plpgsql IMMUTABLE;
>>
>> CREATE SCHEMA foo;
>> SET search_path = foo;
>>
>> CREATE FUNCTION foo() RETURNS text AS $$
>> BEGIN
>>    RETURN bar();
>> END
>> $$ LANGUAGE plpgsql IMMUTABLE;
>>
>> SET search_path = public;
>>
>> CREATE TABLE foobar (d text);
>> insert into foobar (d) values ('foobar');
>>
>> set search_path = public, foo, bar;
>> CREATE INDEX foobar_d on foobar using btree(foo());
>>
>>
>> Run this on a newly created database, and dump it with pg_dump. You'll
>> notice that the dump is unusable.  Creating a new database from this
>> dump will trigger the following error:
>>
>> ERROR:  function bar() does not exist
>> LINE 1: SELECT bar()
>>               ^
>> HINT:  No function matches the given name and argument types. You
>> might need to add explicit type casts.
>> QUERY:  SELECT bar()
>> CONTEXT:  PL/pgSQL function "foo" line 2 at RETURN
>>
>> How can we fix this?
>> --
>> Jean-Baptiste Quenot
>>
>> --
>
> I think Postgres doesn't check to see whether bar() exists in the current
> search path when you create the foo() function, and since it isn't in the
> foo() function's search path value, it fails to find the function when you
> try to use it.  It can probably be fixed (this specific case, not generally)
> with:
>
> ALTER FUNCTION foo.foo() SET search_path=foo, bar;

I suppose that the root of the problem here is that foo() is not
really immutable - it gives different results depending on the search
path.  It seems like that could bite you in a number of different
ways.

I actually wonder if we shouldn't automatically tag plpgsql functions
with the search_path in effect at the time of their creation (as if
the user had done ALTER FUNCTION ... SET search_path=...whatever the
current search path is...).  I suppose the current behavior could
sometimes be useful but on the whole it seems more like a giant
foot-gun which the user oughtn't to get unless they explicitly ask for
it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas <robertmhaas@gmail.com> writes:
> I suppose that the root of the problem here is that foo() is not
> really immutable - it gives different results depending on the search
> path.

Yeah.  The declaration of the function is broken --- it's not pg_dump's
fault that the function misbehaves.

> I actually wonder if we shouldn't automatically tag plpgsql functions
> with the search_path in effect at the time of their creation (as if
> the user had done ALTER FUNCTION ... SET search_path=...whatever the
> current search path is...).

That would be extremely expensive and not very backwards-compatible.
In the case at hand, just writing "RETURN bar.bar();" would be the
best-performing solution.
        regards, tom lane

On Thu, Jun 17, 2010 at 4:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I actually wonder if we shouldn't automatically tag plpgsql functions
>> with the search_path in effect at the time of their creation (as if
>> the user had done ALTER FUNCTION ... SET search_path=...whatever the
>> current search path is...).
>
> That would be extremely expensive and not very backwards-compatible.
> In the case at hand, just writing "RETURN bar.bar();" would be the
> best-performing solution.
>

I wonder if we should have a mode for plpgsql functions where all name
lookups are done at definition time So the bar() function would be
resolved to bar.bar() and stored that way permanently so that pg_dump
dumped the definition as bar.bar().

That would be probably just as good as setting the search path on the
function for most users and better for some. It would have the same
problem with dynamic sql that a lot of things have though.

-- 
greg

2010/6/17 Greg Stark <gsstark@mit.edu>:
> On Thu, Jun 17, 2010 at 4:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I actually wonder if we shouldn't automatically tag plpgsql functions
>>> with the search_path in effect at the time of their creation (as if
>>> the user had done ALTER FUNCTION ... SET search_path=...whatever the
>>> current search path is...).
>>
>> That would be extremely expensive and not very backwards-compatible.
>> In the case at hand, just writing "RETURN bar.bar();" would be the
>> best-performing solution.
>>
>
> I wonder if we should have a mode for plpgsql functions where all name
> lookups are done at definition time So the bar() function would be
> resolved to bar.bar() and stored that way permanently so that pg_dump
> dumped the definition as bar.bar().
>
> That would be probably just as good as setting the search path on the
> function for most users and better for some. It would have the same
> problem with dynamic sql that a lot of things have though.

+1 IMHO PG should dump the bar() function call as bar.bar() to be
safe.  Using fully qualified function name is what I did in my source
code, to work around this problem.
-- 
Jean-Baptiste Quenot

While this problem doesn't result in a unrestorable dump, it does mean manual intervention would be required when perform a restore.  I would expect all backups to restore, so anything like this which causes issue at restore time should really have been caught before it got into the database itself.

What is causing the error is the creation of the index which actually goes to use the function.  In the OP's example, this is deemed okay since it's found in the search path, but in theory, we could end up with the following cases:

CREATE SCHEMA bar;
SET search_path = bar;

/* bar.bar() function */

CREATE FUNCTION bar() RETURNS text AS $$
BEGIN
   RETURN 'foobar';
END
$$ LANGUAGE plpgsql IMMUTABLE;


CREATE SCHEMA foo;
SET search_path = foo;

CREATE FUNCTION foo() RETURNS text AS $$
BEGIN
   RETURN bar();
END
$$ LANGUAGE plpgsql IMMUTABLE;


CREATE SCHEMA moo;
SET search_path = moo;

/* moo.bar() function */

CREATE FUNCTION bar() RETURNS text AS $$
BEGIN
   RETURN 'moobar';
END
$$ LANGUAGE plpgsql IMMUTABLE;


/* CASE 1 */

CREATE TABLE foobar (d text);
insert into foobar (d) values ('foobar');

/* This index will use foo.foo() which in turn will use moo.bar() */
set search_path = foo, moo, bar;
CREATE INDEX foobar_d on foobar using btree(foo.foo());



/* CASE 2 */

CREATE TABLE foobar2 (d text);
insert into foobar2 (d) values ('foobar2');

/* This index will use foo.foo() which in turn will use bar.bar() */
set search_path = foo, bar, moo;
CREATE INDEX foobar2_d on foobar2 using btree(foo.foo());


While both tables are using the same function, they won't have the same functionality due to the difference in the search path at the time of the index creation.  If someone inadvertently ended up with this configuration, it could be very difficult to debug why they have different results.

If functions are parsed and resovled to prefix all contained function calls with the relevant schema, then it would also have to prefix it regardless of whether it's also in the current schema or not as the call could otherwise be misdirected by a search path change prior to the container function's usage.

The alternative, which isn't really an alternative, is to prevent the user from creating a function with references to other functions without a schemaname... which obviously wouldn't be backwards compatible.

Thom