Re: Command Triggers

On Tuesday, February 28, 2012 12:30:36 AM Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > Sorry for letting this slide.
> >
> > Is it worth adding this bit to OpenIntoRel? Not sure if there is danger
> > in allowing anyone to create shared tables
> >
> > /* In all cases disallow placing user relations in pg_global */
> > if (tablespaceId == GLOBALTABLESPACE_OID)
> >
> > ereport(ERROR,
> >
> > (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> >
> > errmsg("only shared relations can be placed in pg_global
> >
> > tablespace")));
>
> Ugh ... if that's currently allowed, we definitely need to fix it.
> But I'm not sure OpenIntoRel is the right place. I'd have expected
> the test to be at some lower level, like heap_create_with_catalog
> or so.
Its definitely allowed right now:

test-upgrade=# CREATE TABLE foo TABLESPACE pg_global AS SELECT 1;
SELECT 1
Time: 354.097 ms

The analogous check for the missing one in OpenIntoRel for plain relations is
in defineRelation. heap_create_with_catalog only contains the inverse check:

/*
* Shared relations must be in pg_global (last-ditch check)
*/
if (shared_relation && reltablespace != GLOBALTABLESPACE_OID)
elog(ERROR, "shared relations must be placed in pg_global
tablespace");

Moving it there sounds like a good idea without any problem I can see right
now. Want me to prepare a patch or is it just the same for you if you do it
yourself?

Andres