Skip site navigation (1) Skip section navigation (2)

Re: Command Triggers

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>
Subject: Re: Command Triggers
Date: 2012-03-20 16:02:01
Message-ID: 201203201702.02130.andres@anarazel.de (view raw or flat)
Thread:
Lists: pgsql-hackers
On Tuesday, February 28, 2012 12:43:02 AM Andres Freund wrote:
> 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?
Sorry to bother you with that dreary topic further, but this should probably 
be fixed before the next set of stable releases.

The check cannot easily be moved to heap_create_with_catalog because e.g. 
cluster.c's make_new_heap does heap_create_with_catalog for a temporary copy 
of shared relations without being able to mark them as such (because they 
don't have the right oid and thus IsSharedRelation would cry). So I think just 
adding the same check to the ctas code as the normal DefineRelation contains 
is the best way forward.

The attached patch applies from 8.3 to 9.1 (8.2 has conflicts but 
thankfully...).

Andres

Attachment: 0001-Check-that-the-specified-tablespace-in-a-CREATE-TABL.patch
Description: text/x-patch (1.6 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Alvaro HerreraDate: 2012-03-20 16:05:25
Subject: Re: Regarding column reordering project for GSoc 2012
Previous:From: Robert HaasDate: 2012-03-20 15:57:40
Subject: Re: vacuumlo issue

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group