Обсуждение: Re: [COMMITTERS] pgsql: Sequences were not being shown due to the use of lowercase 's'

momjian@postgresql.org (Bruce Momjian) writes:
> Sequences were not being shown due to the use of lowercase 's' instead
> of 'S', and the views were not checking for table visibility with
> regards to temporary tables and sequences.

What became of my objection that the test should be on USAGE privilege
for the containing schema instead?
        regards, tom lane

Tom Lane wrote:
> momjian@postgresql.org (Bruce Momjian) writes:
> > Sequences were not being shown due to the use of lowercase 's' instead
> > of 'S', and the views were not checking for table visibility with
> > regards to temporary tables and sequences.
> 
> What became of my objection that the test should be on USAGE privilege
> for the containing schema instead?

I remember puzzling over Greg's reply:
http://archives.postgresql.org/pgsql-patches/2006-08/msg00247.php

Anyway, Greg is going to fix that, plus the syntax error in his other
patch.  I will see it gets corrected.

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


I wrote:
> Sequences were not being shown due to the use of lowercase 's' instead
> of 'S', and the views were not checking for table visibility with
> regards to temporary tables and sequences.

Tom Lane replied:
>> What became of my objection that the test should be on USAGE privilege
>> for the containing schema instead?

I took a stab at implementing this, but what exactly would we check? Looks
like all the temp tables have automatic usage for the same user, according to

SELECT *,has_schema_privilege(oid,'USAGE') FROM pg_namespace;

So I'd need another way to test that the schema was created by another process.
I agree that is_visible may not be ideal for most cases, but it should be okay
if we are simply using it to filter temporary schemas, right?

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation
PGP Key: 0x14964AC8 200609041803
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iD8DBQFE/Km6vJuQZxSWSsgRAgkaAKC/Nzc8xIcxRC1TW2UJCB76LurWmgCg+Dkk
4HbMsy4H1uwRAUz9lqCSdXg=
=eBg2
-----END PGP SIGNATURE-----

"Greg Sabino Mullane" <greg@turnstep.com> writes:
> Tom Lane replied:
>> What became of my objection that the test should be on USAGE privilege
>> for the containing schema instead?

> I took a stab at implementing this, but what exactly would we check? Looks
> like all the temp tables have automatic usage for the same user, according to

> SELECT *,has_schema_privilege(oid,'USAGE') FROM pg_namespace;

Well, if you test it as a superuser, it's going to return TRUE every
time.
        regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


> SELECT *,has_schema_privilege(oid,'USAGE') FROM pg_namespace;

>> Well, if you test it as a superuser, it's going to return TRUE every
>> time.

Exactly. So I'm not seeing how we can use USAGE as a reliable test for
the case where a temporary table was created by the same user, but in
another session.

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation
PGP Key: 0x14964AC8 200609041941
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iD8DBQFE/LsJvJuQZxSWSsgRAt5mAKDWAWmnljELeRJn+LvdAnpfkwhDIwCfSls8
hR0xST8C88uA4xXrEP6pAh0=
=bHRd
-----END PGP SIGNATURE-----

"Greg Sabino Mullane" <greg@turnstep.com> writes:
>>> SELECT *,has_schema_privilege(oid,'USAGE') FROM pg_namespace;

>> Well, if you test it as a superuser, it's going to return TRUE every
>> time.

> Exactly. So I'm not seeing how we can use USAGE as a reliable test for
> the case where a temporary table was created by the same user, but in
> another session.

Superusers can access anything they want to.  What's your point?
The spec says "accessible" ...
        regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Tom Lane asked:
> Superusers can access anything they want to.  What's your point?
> The spec says "accessible" ...

<disclaimer>
Not trying to lecture you Tom :), just posting my argument
here for others.
</disclaimer>

Temp tables are "special" because the user does not know (and,
more importantly, should not usually have to know) which
pg_temp_ schema the table is created in. For example, if I am
in session #1 and create a table, I simply issue

CREATE TABLE foobar(a int);

If I want to test for the table's existence, I simply do:

SELECT 1 FROM information_schema.tables
WHERE table_name ='foobar';

If I want to be more specific with regards to a schema:

CREATE TABLE zoo.foobar(a int);

SELECT 1 FROM information_schema.tables
WHERE table_name ='foobar' AND table_schema = 'zoo';

However, if I create a temp table, a problem occurs:

CREATE TEMP TABLE foobar(a int);

SELECT 1 FROM information_schema.tables
WHERE table_name ='foobar'; -- which schema?

The above might give a false positive if another session has
created a temporary table of that name. Since the whole point of
temp tables is temporary per-session relations, it seems silly
for information_schema to tell me that another session already
has a temporary table by that name, since that information has
no use to me whatsoever. I cannot read from the other temp table
(which could be a strong "non-accessible" argument), and its
existence won't stop me from creating a same-named temporary table
in my own session. The only thing it can do is cause errors for
people who think that there is already a temporary table by that
name and try to drop it (which is what prompted this patch in
the first place).

I can't think of a use case where a user would not want to
append a "is_visible" clause to the query above. That or start
tracking which pg_temp_ schema belongs to whom.

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation
PGP Key: 0x14964AC8 200609061927
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iD8DBQFE/1unvJuQZxSWSsgRAgC3AJ4kNmy2DMdGcZmsnbfAkODahKIgTACg9q2I
+5q4E6BDmU87o28DnG5QZ1s=
=4GFl
-----END PGP SIGNATURE-----

"Greg Sabino Mullane" <greg@turnstep.com> writes:
> ... I can't think of a use case where a user would not want to
> append a "is_visible" clause to the query above. That or start
> tracking which pg_temp_ schema belongs to whom.

Well, I'm still having a problem with this, because it seems like a
pretty klugy solution.  It's inefficient (the is_visible functions
are not cheap) and it's not hard to fool:
set search_path = pg_temp_N, ...

(This won't work for a non-superuser, because he'll not have USAGE
privilege on someone else's temp schema, but you seem to be worried
about hiding temp tables from superusers.)

If you're really intent on making it work this way, my vote is to
expose namespace.c's isOtherTempNamespace() as a SQL-callable function,
and add a test on that to the info-schema views, rather than relying on
is_visible or explicit knowledge of the temp-schema naming convention.

Perhaps we should expose bothpg_is_my_temp_schema(schema_oid)pg_is_other_temp_schema(schema_oid)

Thoughts?  Opinions about the function names?
        regards, tom lane

I wrote:
> If you're really intent on making it work this way, my vote is to
> expose namespace.c's isOtherTempNamespace() as a SQL-callable function,
> and add a test on that to the info-schema views, rather than relying on
> is_visible or explicit knowledge of the temp-schema naming convention.

I've done the above and now withdraw my complaints about this patch.
I notice however that the patch seems to have touched only about half a
dozen of the information_schema views ... shouldn't more of them have
similar filters?
        regards, tom lane

Tom Lane wrote:
> momjian@postgresql.org (Bruce Momjian) writes:
> > Sequences were not being shown due to the use of lowercase 's' instead
> > of 'S', and the views were not checking for table visibility with
> > regards to temporary tables and sequences.
> 
> What became of my objection that the test should be on USAGE privilege
> for the containing schema instead?

Was this addressed?

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> What became of my objection that the test should be on USAGE privilege
>> for the containing schema instead?

> Was this addressed?

Yes, we arrived at this:
http://archives.postgresql.org/pgsql-committers/2006-09/msg00252.php
which does what Greg wanted but without the kluges.
        regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Tom Lane wrote:
> I've done the above and now withdraw my complaints about this patch.

Excellent, thank you.

> I notice however that the patch seems to have touched only about half a
> dozen of the information_schema views ... shouldn't more of them have
> similar filters?

Probably. I did this mainly as a fix to the .tables view, and added what
appeared to be other logical places, but it's entirely likely that
[almost] all of them need it. I can whip up a new patch based on the
new functions if you haven't done the work already. :)

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation
PGP Key: 0x14964AC8 200609181159
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iD8DBQFFDsKgvJuQZxSWSsgRAn70AKCwa2jePz+wyJdFqzMvgqjYkRWFDwCeK6s/
TXJAHKinmRYXfT8o8eV1PYc=
=f5vT
-----END PGP SIGNATURE-----