Thread: creating objects in pg_catalog
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
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
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
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
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
Attachment
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