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 <and...@anarazel.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
From 77896a7385d1ef5c793e06c5085a8b37ab5857c9 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 20 Mar 2012 16:33:13 +0100
Subject: [PATCH] Check that the specified tablespace in a CREATE TABLE AS
command is not pg_global
That check was not added to the CTAS code when it was added to the ordinary
CREATE TABLE AS.
It might be nicer to add that check heap_create_with_catalog as well, but thats
not easily possible because e.g. cluster creates a temporary new heap which
cannot be marked shared because there is a fixed list of shared relations (see
IsSharedRelation).
---
src/backend/executor/execMain.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 621ad8a..8a43db7 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -43,6 +43,7 @@
#include "access/xact.h"
#include "catalog/heap.h"
#include "catalog/namespace.h"
+#include "catalog/pg_tablespace.h"
#include "catalog/toasting.h"
#include "commands/tablespace.h"
#include "commands/trigger.h"
@@ -2452,6 +2453,12 @@ OpenIntoRel(QueryDesc *queryDesc)
get_tablespace_name(tablespaceId));
}
+ /* 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")));
+
/* Parse and validate any reloptions */
reloptions = transformRelOptions((Datum) 0,
into->options,
--
1.7.6.409.ge7a85.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
