Обсуждение: public.pg_class isn't a catalog table

Version 1.6.3; if you have a table in public called pg_class and try
to drop it, pgAdmin gives an error:

ERROR: permission denied: "pg_class" is a system catalog

The drop does work correctly from psql...
--
Decibel!, aka Jim Nasby                        decibel@decibel.org
EnterpriseDB      http://enterprisedb.com      512.569.9461 (cell)

Decibel! wrote:

> Version 1.6.3; if you have a table in public called pg_class and try to
> drop it, pgAdmin gives an error:
>
> ERROR: permission denied: "pg_class" is a system catalog
>
That's because pgAdmin is issuing "DROP TABLE pg_class" instead of "DROP
TABLE public.pg_class". By the way, is there any reason for not using
the namespace?
I don't know if it's the best way but this little proof of concept fix
might be applied to all objects that can be created inside a schema. I
could provide I patch if you want it.


--
  Euler Taveira de Oliveira
  http://www.timbira.com/
Index: pgadmin/schema/pgTable.cpp
===================================================================
--- pgadmin/schema/pgTable.cpp    (revis�o 6614)
+++ pgadmin/schema/pgTable.cpp    (c�pia de trabalho)
@@ -126,7 +126,13 @@

 bool pgTable::DropObject(wxFrame *frame, ctlTree *browser, bool cascaded)
 {
-    wxString sql = wxT("DROP TABLE ") + GetQuotedFullIdentifier();
+    wxString sql;
+
+    if (this->GetSchema()->GetName() == this->GetDatabase()->GetDefaultSchema())
+        sql = wxT("DROP TABLE ") + this->GetSchema()->GetName() + wxT(".") + GetQuotedFullIdentifier();
+    else
+        sql = wxT("DROP TABLE ") + GetQuotedFullIdentifier();
+
     if (cascaded)
         sql += wxT(" CASCADE");
     return GetDatabase()->ExecuteVoid(sql);

Euler Taveira de Oliveira a écrit :
> Decibel! wrote:
>
>> Version 1.6.3; if you have a table in public called pg_class and try to
>> drop it, pgAdmin gives an error:
>>
>> ERROR: permission denied: "pg_class" is a system catalog
>>
> That's because pgAdmin is issuing "DROP TABLE pg_class" instead of "DROP
> TABLE public.pg_class". By the way, is there any reason for not using
> the namespace?
> I don't know if it's the best way but this little proof of concept fix
> might be applied to all objects that can be created inside a schema. I
> could provide I patch if you want it.
>

Thanks for your patch. I took a look at it and I wonder why you check if
the object's schema is the default one. I mean, we just need to add the
object's schema and we're done, aren't we ?

And do we have a GetQuotedName method for the schema ? if yes, it would
be better to use it.

Regards.


--
Guillaume.
<!-- http://abs.traduc.org/
     http://lfs.traduc.org/
     http://docs.postgresqlfr.org/ -->

Euler Taveira de Oliveira wrote:
> Decibel! wrote:
>
>> Version 1.6.3; if you have a table in public called pg_class and
>> try to drop it, pgAdmin gives an error:
>>
>> ERROR: permission denied: "pg_class" is a system catalog
>>
> That's because pgAdmin is issuing "DROP TABLE pg_class" instead of
> "DROP TABLE public.pg_class". By the way, is there any reason for not
> using the namespace?

It's cosmetic, for when the reverse engineered SQL is generated. Most
people prefer not to see the schema prefixed when it's not needed. Of
course, that's no reason to mess around with hidden SQL to drop the object.

This falls over when the user creates objects with the same name as
those in pg_catalog of course. The normal response is 'Don't do that!'

> I don't know if it's the best way but this little proof of concept
> fix might be applied to all objects that can be created inside a
> schema. I could provide I patch if you want it.

The patch is essentially reversing the logic to prevents the schema
being put there in the first place (which happens somewhere down the
stack under GetQuotedFullIdentifier). It would be a little cleaner just
to always construct the name manually.

For the record, here's some clarification on some of the schema member
functions:

->GetName()    // Returns the name of the object

->GetIdentifier()    // Returns the identifier - e.g. for a function, this
is the name and signature

->GetQuotedName()    // Returns the name, quoted if necessary\

->GetQuotedIdentifier()    // Returns the identifier, quoted if necessary

->GetQuotedFullIdentifier()    // Returns the quoted identifier, with a
quoted schema prefix, *if* the prefix is required.

I'll commit the following new SQL to pgTable::DropObject:

wxString sql = wxT("DROP TABLE ") +
this->GetSchema()->GetQuotedIdentifier() + wxT(".") +
this->GetQuotedIdentifier();

Regards, Dave

Dave Page wrote:
> ->GetIdentifier()    // Returns the identifier - e.g. for a function, this
> is the name and signature

Hmm, that seems to have been changed at some point. I'm not sure why. I
guess I'll have to have a cleanup in that area...

:-(

/D

Decibel! wrote:
> Version 1.6.3; if you have a table in public called pg_class and try to
> drop it, pgAdmin gives an error:
>
> ERROR: permission denied: "pg_class" is a system catalog
>
> The drop does work correctly from psql...

I'm sure you (Jim) have seen the reasons why this happens pgAdmin in the
rest of the thread, but I wonder also if this could be considered a bug
in the server:

foo=# create table pg_class(a text);
CREATE TABLE
foo=# drop table pg_class;
ERROR:  permission denied: "pg_class" is a system catalog
foo=# drop table public.pg_class;
DROP TABLE
foo=#

It's certainly inconsistent.

/D