Re: erroneous restore into pg_catalog schema

On Sat, May 4, 2013 at 3:59 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Given the lack of any good alternative proposals, I think we should
> press ahead with this approach. We still need doc updates and fixes
> for the affected contrib module(s), though. Also, in view of point 2,
> it seems like the extensions code should test for and reject an attempt
> to set a relocatable extension's schema to pg_catalog. Otherwise you'd
> be likely to get not-too-intelligible errors from the extension script.

I looked into this a bit more. It seems that adminpack is OK: it
already qualifies all of the objects it creates with the pg_catalog
schema. With the previous patch applied, all of the built-in
extensions seem to install OK (except for uuid-ossp which I'm not set
up to build, but it doesn't look like a problem) make check-world
also passes. (adminpack actually has no regression tests, not even a
test that the extension installs, but it does.)

I looked for a suitable place to update the documentation and I don't
have any brilliant ideas. The point that we're going to forbid
relocatable extensions from being created in pg_catalog seems too
minor to be worth noting; we don't document every trivial error
message that can occur for every command in the manual. The point
that we won't create ANY objects in pg_catalog unless the CREATE
statement schema-qualifies the object name seems like it would be
worth pointing out, but I don't see an obvious place to do it.
Suggestions?

In the attached new version of the patch, I added an explicit check to
prevent relocatable extensions from being created in pg_catalog.
Along the way, I noticed something else: these changes mean that
fetch_search_path's includeImplicit flag doesn't really quite do what
it says any more. I'm not sure whether we should change the behavior
or rename the flag to, say, creationTargetsOnly. For the extension
machinery's purpose, the existing meaning of the flag matches what it
wants, but I replaced a couple of elog() calls with ereport(), using
an existing message text; I suspect you could hit one or both of these
before, and you certainly can with these changes.

The other places where fetch_search is used with an argument of false
are in the SQL functions current_schema() and current_schemas(). This
change will cause them not to return pg_catalog in some cases where
they currently would. It is unclear to me whether that is a good
thing or a bad thing.

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