Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Magnus Hagander
Tom Lane wrote:
 Magnus Hagander [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 * Just remove the above-quoted lines.  Superusers should be allowed to
 shoot themselves in the foot.  (I'm not actually sure that there would
 be any bad consequences from putting an ordinary table into pg_global
 anyway.
 
 Is there ever *any* reason for doing this?
 
 Probably not a good one, and I suspect there would be some funny
 misbehaviors if you were to clone the database containing the table.
 The table would be physically shared but logically not.

yuck.


 What I'm inclined to do about it is is adopt my suggestion #2 (move the
 location of the defense), since permission denied for a superuser is
 a pretty unhelpful error message anyway.

Ok. Works for me.


 * Decide that we should allow anyone to do pg_tablespace_size('pg_global')
 and put in a special wart for that in dbsize.c.  This wasn't part of
 the original agreement but maybe there's a case to be made for it.
 
 That's pretty much the same thing, right?
 
 Well, no, I was suggesting that we might want to special-case pg_global
 as a tablespace that anyone (superuser or no) could get the size of.
 This is actually independent of whether we change the aclmask behavior.

Oh, ok, I see. Then my vote is for the other solution = not allowing
anybody to do this.

//Magnus

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Tom Lane
Magnus Hagander [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 * Just remove the above-quoted lines.  Superusers should be allowed to
 shoot themselves in the foot.  (I'm not actually sure that there would
 be any bad consequences from putting an ordinary table into pg_global
 anyway.

 Is there ever *any* reason for doing this?

Probably not a good one, and I suspect there would be some funny
misbehaviors if you were to clone the database containing the table.
The table would be physically shared but logically not.

What I'm inclined to do about it is is adopt my suggestion #2 (move the
location of the defense), since permission denied for a superuser is
a pretty unhelpful error message anyway.

 * Decide that we should allow anyone to do pg_tablespace_size('pg_global')
 and put in a special wart for that in dbsize.c.  This wasn't part of
 the original agreement but maybe there's a case to be made for it.

 That's pretty much the same thing, right?

Well, no, I was suggesting that we might want to special-case pg_global
as a tablespace that anyone (superuser or no) could get the size of.
This is actually independent of whether we change the aclmask behavior.

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Magnus Hagander
Tom Lane wrote:
 I wrote:
 [ squint... ]  There is something wrong here, because a superuser should
 certainly pass the aclcheck test.  I don't know where the bug is but
 this is not the correct fix.
 
 OK, after looking, the issue is this wart in pg_tablespace_aclmask():
 
 /*
  * Only shared relations can be stored in global space; don't let even
  * superusers override this
  */
 if (spc_oid == GLOBALTABLESPACE_OID  !IsBootstrapProcessingMode())
 return 0;
 
 /* Otherwise, superusers bypass all permission checking. */

Yup, that was my point.


 There are a number of ways that we could deal with this:
 
 * Just remove the above-quoted lines.  Superusers should be allowed to
 shoot themselves in the foot.  (I'm not actually sure that there would
 be any bad consequences from putting an ordinary table into pg_global
 anyway.  I think I wrote the above code in fear that some parts of the
 system would equate reltablespace = pg_global with relisshared, but
 AFAICS that's not the case now.)

Is there ever *any* reason for doing this?
If there isn't, I don't think we should provide just that foot-gun. But
if there is any case where it makes sense to do that, then the superuser
should probably be allowed to do it.


 * Remove the above lines and instead put a defense into heap_create.
 This might be better design anyway since a more specific error could
 be reported.
 
 * Leave aclchk.c as-is and apply Magnus' patch to allow superusers
 to bypass the check in pg_tablespace_size.

See foot-gun above. If we want to keep the check, I think that my patch
is fine. If we don't, then taking out that code is better.


 * Decide that we should allow anyone to do pg_tablespace_size('pg_global')
 and put in a special wart for that in dbsize.c.  This wasn't part of
 the original agreement but maybe there's a case to be made for it.

That's pretty much the same thing, right? Since the acl check will check
for pg_global, and if it's anything else, let superuser in. It's gotta
be easier to read if it's just a plain superuser check, I think.

//Magnus

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Magnus Hagander
Tom Lane wrote:
 Magnus Hagander [EMAIL PROTECTED] writes:
 Attached is a patch I want to apply for this. Toms message at
 http://archives.postgresql.org/pgsql-hackers/2007-10/msg00448.php makes me
 bring it up here before I apply it.
 
 [ squint... ]  There is something wrong here, because a superuser should
 certainly pass the aclcheck test.  I don't know where the bug is but
 this is not the correct fix.

Are you sure? pg_tablespace_aclmask() has:
/*
 * Only shared relations can be stored in global space; don't let even
 * superusers override this
 */
if (spc_oid == GLOBALTABLESPACE_OID  !IsBootstrapProcessingMode())
return 0;


And this is what causes the failure. I certainly didn't want to take out
that check, so short-circuiting it in the way I did seemed right..

//Magnus


---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Bruce Momjian
Tom Lane wrote:
 Dave Page [EMAIL PROTECTED] writes:
  select pg_tablespace_size('pg_global');
  ERROR:  permission denied for tablespace pg_global
 
  Can this be fixed please?
 
 What's to be fixed?  That's exactly the result I'd expect given the
 previously-agreed-to behavior for pg_tablespace_size.

I know we said tablespace requires CREATE privs, but certainly the
super-user should be able to use the function too.

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://postgres.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Tom Lane
Magnus Hagander [EMAIL PROTECTED] writes:
 Attached is a patch I want to apply for this. Toms message at
 http://archives.postgresql.org/pgsql-hackers/2007-10/msg00448.php makes me
 bring it up here before I apply it.

[ squint... ]  There is something wrong here, because a superuser should
certainly pass the aclcheck test.  I don't know where the bug is but
this is not the correct fix.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Magnus Hagander
On Fri, Oct 12, 2007 at 12:20:08PM +0100, Dave Page wrote:
 Following on from Hiroshi's earlier post:
 http://archives.postgresql.org/pgsql-hackers/2007-10/msg00324.php
 
 Per http://archives.postgresql.org/pgsql-hackers/2007-08/msg01018.php,
 superusers should be able to use pg_tablespace_size() on any tablespace,
 however when logged in as postgres on a beta1 server, I get:
 
 -- Executing query:
 select pg_tablespace_size('pg_global');
 
 ERROR:  permission denied for tablespace pg_global
 
 Can this be fixed please?

Attached is a patch I want to apply for this. Toms message at
http://archives.postgresql.org/pgsql-hackers/2007-10/msg00448.php makes me
bring it up here before I apply it.

I think the correct behaviour is that superusers should be able to do it,
and that's in the thread that Dave referred to. But since Tom said it's by
design in the message above, I'd like confirmation that this is ok.

Since I think that allowing superusers to do it is the only sensible thing,
I will apply this patch unless someone objects :-)

//Magnus
Index: src/backend/utils/adt/dbsize.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/dbsize.c,v
retrieving revision 1.14
diff -c -r1.14 dbsize.c
*** src/backend/utils/adt/dbsize.c  29 Aug 2007 17:24:29 -  1.14
--- src/backend/utils/adt/dbsize.c  12 Oct 2007 14:38:32 -
***
*** 164,171 
/*
 * User must have CREATE privilege for target tablespace, either 
explicitly
 * granted or implicitly because it is default for current database.
 */
!   if (tblspcOid != MyDatabaseTableSpace)
{
aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), 
ACL_CREATE);
if (aclresult != ACLCHECK_OK)
--- 164,175 
/*
 * User must have CREATE privilege for target tablespace, either 
explicitly
 * granted or implicitly because it is default for current database.
+*
+* Specifically check for superuser permissions here, since
+* pg_tablespace_aclcheck() will deny access to pg_global even for
+* superusers.
 */
!   if (tblspcOid != MyDatabaseTableSpace  !superuser())
{
aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), 
ACL_CREATE);
if (aclresult != ACLCHECK_OK)

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


[HACKERS] pg_tablespace_size()

2007-10-12 Thread Dave Page
Following on from Hiroshi's earlier post:
http://archives.postgresql.org/pgsql-hackers/2007-10/msg00324.php

Per http://archives.postgresql.org/pgsql-hackers/2007-08/msg01018.php,
superusers should be able to use pg_tablespace_size() on any tablespace,
however when logged in as postgres on a beta1 server, I get:

-- Executing query:
select pg_tablespace_size('pg_global');

ERROR:  permission denied for tablespace pg_global

Can this be fixed please?

Regards, Dave

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Tom Lane
I wrote:
 [ squint... ]  There is something wrong here, because a superuser should
 certainly pass the aclcheck test.  I don't know where the bug is but
 this is not the correct fix.

OK, after looking, the issue is this wart in pg_tablespace_aclmask():

/*
 * Only shared relations can be stored in global space; don't let even
 * superusers override this
 */
if (spc_oid == GLOBALTABLESPACE_OID  !IsBootstrapProcessingMode())
return 0;

/* Otherwise, superusers bypass all permission checking. */

There are a number of ways that we could deal with this:

* Just remove the above-quoted lines.  Superusers should be allowed to
shoot themselves in the foot.  (I'm not actually sure that there would
be any bad consequences from putting an ordinary table into pg_global
anyway.  I think I wrote the above code in fear that some parts of the
system would equate reltablespace = pg_global with relisshared, but
AFAICS that's not the case now.)

* Remove the above lines and instead put a defense into heap_create.
This might be better design anyway since a more specific error could
be reported.

* Leave aclchk.c as-is and apply Magnus' patch to allow superusers
to bypass the check in pg_tablespace_size.

* Decide that we should allow anyone to do pg_tablespace_size('pg_global')
and put in a special wart for that in dbsize.c.  This wasn't part of
the original agreement but maybe there's a case to be made for it.

Thoughts?

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Tom Lane
Dave Page [EMAIL PROTECTED] writes:
 In the message I quoted at the end of the long discussion on the topic
 you assured me it would work for superusers.
 Is there any reason the superuser shouldn't see the size of pg_global?

Oh, you were superuser?  Then something's broken, agreed.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Dave Page
Tom Lane wrote:
 Dave Page [EMAIL PROTECTED] writes:
 select pg_tablespace_size('pg_global');
 ERROR:  permission denied for tablespace pg_global
 
 Can this be fixed please?
 
 What's to be fixed?  That's exactly the result I'd expect given the
 previously-agreed-to behavior for pg_tablespace_size.

In the message I quoted at the end of the long discussion on the topic
you assured me it would work for superusers.

Is there any reason the superuser shouldn't see the size of pg_global?

Regards, Dave.


---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [HACKERS] pg_tablespace_size()

2007-10-12 Thread Tom Lane
Dave Page [EMAIL PROTECTED] writes:
 select pg_tablespace_size('pg_global');
 ERROR:  permission denied for tablespace pg_global

 Can this be fixed please?

What's to be fixed?  That's exactly the result I'd expect given the
previously-agreed-to behavior for pg_tablespace_size.

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate