Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Checking to decide whether relations are system relations now depends
- 9999f5a10e72 7.3.1 cited

creating objects in pg_catalog

Robert Haas <robertmhaas@gmail.com> — 2012-06-06T18:35:38Z

Right now, you can't directly create a relation (table, index,
composite type) in the pg_catalog schema, but you can create a
non-relation (function, domain, etc.) in the pg_catalog schema.
Furthermore, you can create a table in some other schema and then move
it into the pg_catalog schema using ALTER TABLE .. SET SCHEMA.  After
you do that, you can't move it back out again, nor can you drop it; or
at least not without setting allow_system_table_mods.

This all seems pretty wonky and inconsistent to me.  It strikes me
that we ought to either (1) allow users to place SQL objects in
pg_catalog or (2) not.  Having a weird special case that disallows it
only for relations, but then lets you do it anyway via the back door,
seems pretty pointless.

Tabula raza, I'd argue for getting tough on this, and error out on any
attempt to get a user-created SQL object into pg_catalog by any means,
unless allow_system_table_mods is set.  However, considering that we
have two extensions whose extension install scripts do this --
adminpack and sepgsql -- and one of those (adminpack) is extremely
widely used, that seems like it might be asking for trouble.  So maybe
we should just go the opposite direction and just remove the rather
toothless prohibition that currently exists.  Or, as a middle way, we
could tighten up the prohibition, but also provide a GUC other than
allow_system_table_mods that can be changed via SET LOCAL by extension
scripts that need to do this.  allow_system_table_mods requires a
server restart and is purposefully undocumented, so it's not a good
thing to rely on for prohibitions that people might need to work
around on a production system.

Thoughts?

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

Re: creating objects in pg_catalog

Tom Lane <tgl@sss.pgh.pa.us> — 2012-06-06T20:39:42Z

Robert Haas <robertmhaas@gmail.com> writes:
> Right now, you can't directly create a relation (table, index,
> composite type) in the pg_catalog schema, but you can create a
> non-relation (function, domain, etc.) in the pg_catalog schema.

Surely this is true only for superusers.  Superusers can do whatever
they want anyway, no?

> Tabula raza, I'd argue for getting tough on this, and error out on any
> attempt to get a user-created SQL object into pg_catalog by any means,
> unless allow_system_table_mods is set.

allow_system_table_mods is mainly intended to prevent people from
altering the schemas of system catalogs, since it's more than likely
that the backend C code will fail (nastily) to cope with such changes.
I don't think it follows that we should prevent superusers from doing
things that are perfectly safe, like adding new functions in pg_catalog.

It's very likely that the specific restrictions enforced by
allow_system_table_mods could stand a fresh look, but I don't agree
with the idea of radically changing its charter.  Nor do I think we
need to put training wheels on superusers for any changes that aren't
demonstrably likely to result in unrecoverable database corruption.

			regards, tom lane

Re: creating objects in pg_catalog

Robert Haas <robertmhaas@gmail.com> — 2012-06-06T21:09:31Z

On Wed, Jun 6, 2012 at 4:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Right now, you can't directly create a relation (table, index,
>> composite type) in the pg_catalog schema, but you can create a
>> non-relation (function, domain, etc.) in the pg_catalog schema.
>
> Surely this is true only for superusers.  Superusers can do whatever
> they want anyway, no?

No.

rhaas=# create table pg_catalog.tom (a int);
ERROR:  permission denied to create "pg_catalog.tom"
DETAIL:  System catalog modifications are currently disallowed.
rhaas=# create table tom (a int);
CREATE TABLE
rhaas=# alter table tom set schema pg_catalog;
ALTER TABLE
rhaas=# create domain pg_catalog.lane as int;
CREATE DOMAIN

The offending error check is in heap_create(), and based on what
you're saying here it seems like we should just rip it out.

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

Re: creating objects in pg_catalog

Tom Lane <tgl@sss.pgh.pa.us> — 2012-06-06T21:21:35Z

Robert Haas <robertmhaas@gmail.com> writes:
> rhaas=# create table pg_catalog.tom (a int);
> ERROR:  permission denied to create "pg_catalog.tom"

> The offending error check is in heap_create(), and based on what
> you're saying here it seems like we should just rip it out.

Hmm.  Yeah, it seems like the regular permissions tests on the schemas
in question should be enough to keep Joe User from making tables there,
and I do not see a reason why the backend would care if there are
non-catalog tables laying about in pg_catalog.

Checking the commit history, it seems this was originally a test to
prevent people from creating tables named "pg_xxx":

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9999f5a10e722c052006886b678995695001958a#patch3

which may or may not have been critical once upon a time, but surely is
not any more.

So no objection to removing that particular test.

			regards, tom lane

Re: creating objects in pg_catalog

Robert Haas <robertmhaas@gmail.com> — 2012-06-07T12:44:26Z

On Wed, Jun 6, 2012 at 5:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> rhaas=# create table pg_catalog.tom (a int);
>> ERROR:  permission denied to create "pg_catalog.tom"
>
>> The offending error check is in heap_create(), and based on what
>> you're saying here it seems like we should just rip it out.
>
> Hmm.  Yeah, it seems like the regular permissions tests on the schemas
> in question should be enough to keep Joe User from making tables there,
> and I do not see a reason why the backend would care if there are
> non-catalog tables laying about in pg_catalog.
>
> Checking the commit history, it seems this was originally a test to
> prevent people from creating tables named "pg_xxx":
>
> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9999f5a10e722c052006886b678995695001958a#patch3
>
> which may or may not have been critical once upon a time, but surely is
> not any more.
>
> So no objection to removing that particular test.

Patch attached.  Barring objections, I'll apply this to 9.3 once we branch.

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

Re: creating objects in pg_catalog

Robert Haas <robertmhaas@gmail.com> — 2012-06-14T14:02:20Z

On Thu, Jun 7, 2012 at 8:44 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jun 6, 2012 at 5:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> rhaas=# create table pg_catalog.tom (a int);
>>> ERROR:  permission denied to create "pg_catalog.tom"
>>
>>> The offending error check is in heap_create(), and based on what
>>> you're saying here it seems like we should just rip it out.
>>
>> Hmm.  Yeah, it seems like the regular permissions tests on the schemas
>> in question should be enough to keep Joe User from making tables there,
>> and I do not see a reason why the backend would care if there are
>> non-catalog tables laying about in pg_catalog.
>>
>> Checking the commit history, it seems this was originally a test to
>> prevent people from creating tables named "pg_xxx":
>>
>> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9999f5a10e722c052006886b678995695001958a#patch3
>>
>> which may or may not have been critical once upon a time, but surely is
>> not any more.
>>
>> So no objection to removing that particular test.
>
> Patch attached.  Barring objections, I'll apply this to 9.3 once we branch.

Hearing no objections, done.

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