Re: pg_tablespace_size()

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