Обсуждение: [7.4] "permissions problem" with pl/pgsql function

Just had a client point this out to me, and am wondering if its supposed
to happen:

420_test=> select
dropgeometrycolumn('420_test','lroadline61','roads61_geom');
ERROR:  permission denied for relation pg_attribute
CONTEXT:  PL/pgSQL function "dropgeometrycolumn" line 19 at execute statement

the database was created as:

CREATE DATABASE <db> WITH OWNER = <owner>

and I'm connected to the database as the owner ... shouldn't the "system
tables" also be owned by the owner?


----
Marc G. Fournier           Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org           Yahoo!: yscrappy              ICQ: 7615664

"Marc G. Fournier" <scrappy@hub.org> writes:
> Just had a client point this out to me, and am wondering if its supposed
> to happen:

> 420_test=> select
> dropgeometrycolumn('420_test','lroadline61','roads61_geom');
> ERROR:  permission denied for relation pg_attribute
> CONTEXT:  PL/pgSQL function "dropgeometrycolumn" line 19 at execute statement

Can't tell much about this without seeing the contents of the function ...
in particular, what SQL command is it trying to execute when it chokes?
        regards, tom lane

> 420_test=> select
> dropgeometrycolumn('420_test','lroadline61','roads61_geom');
> ERROR:  permission denied for relation pg_attribute
> CONTEXT:  PL/pgSQL function "dropgeometrycolumn" line 19 at execute statement
> 
> the database was created as:
> 
> CREATE DATABASE <db> WITH OWNER = <owner>
> 
> and I'm connected to the database as the owner ... shouldn't the "system
> tables" also be owned by the owner?

No, you have to have the usecatupd field set to true in your pg_shadow 
row to be able to modify the catalogs.  This is automatically assigned 
to a superuser, not the database owner.  (Otherwise it's trivial to munt 
someone else's database by deleting from pg_database or pg_shadow...)

Chris

On Mon, 19 Jan 2004, Tom Lane wrote:

> "Marc G. Fournier" <scrappy@hub.org> writes:
> > Just had a client point this out to me, and am wondering if its supposed
> > to happen:
>
> > 420_test=> select
> > dropgeometrycolumn('420_test','lroadline61','roads61_geom');
> > ERROR:  permission denied for relation pg_attribute
> > CONTEXT:  PL/pgSQL function "dropgeometrycolumn" line 19 at execute statement
>
> Can't tell much about this without seeing the contents of the function ...
> in particular, what SQL command is it trying to execute when it chokes?

The function is executing:
EXECUTE ''update pg_attribute set attnotnull = false from pg_class where                 pg_attribute.attrelid =
pg_class.oidand pg_class.relname = '' ||                 quote_literal(table_name) ||'' and pg_attribute.attname = ''
||                quote_literal(column_name);
 


I'm going to hit up the PostGis folks, since right at the top of the
function it stats:

-- There is no ALTER TABLE DROP COLUMN command in postgresql
-- There is no ALTER TABLE DROP CONSTRAINT command in postgresql
-- So, we:
--   1. remove the unwanted geom column reference from the
--      geometry_columns table
--   2. update the table so that the geometry column is all NULLS
--      This is okay since the CHECK srid(geometry) = <srid> is not
--      checked if geometry is NULL (the isstrict attribute on srid())
--   3. add another constraint that the geometry column must be NULL
-- This, effectively kills the geometry column
--   (a) its not in the geometry_column table
--   (b) it only has nulls in it
--   (c) you cannot add anything to the geom column because it must be NULL
-- 
-- This will screw up if you put a NOT NULL constraint on the geometry
-- column, so the first thing we must do is remove this constraint (its a
-- modification of the pg_attribute system table)
--
-- We also check to see if the table/column exists in the geometry_columns
-- table

Anyone on this list working with the PostGis development team?

----
Marc G. Fournier           Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org           Yahoo!: yscrappy              ICQ: 7615664

"Marc G. Fournier" <scrappy@postgresql.org> writes:
> On Mon, 19 Jan 2004, Tom Lane wrote:
>> in particular, what SQL command is it trying to execute when it chokes?

> The function is executing:

>     EXECUTE ''update pg_attribute set attnotnull = false from pg_class where
>                   pg_attribute.attrelid = pg_class.oid and pg_class.relname = '' ||
>                   quote_literal(table_name) ||'' and pg_attribute.attname = '' ||
>                   quote_literal(column_name);

Well, no wonder.  You have to be superuser to do that, and it's a pretty
bad idea even then.

We do have ALTER TABLE ... SET/DROP NOT NULL since 7.3, so hacking
pg_attribute directly isn't necessary for this anymore.
        regards, tom lane

On Tue, 20 Jan 2004, Tom Lane wrote:

> "Marc G. Fournier" <scrappy@postgresql.org> writes:
> > On Mon, 19 Jan 2004, Tom Lane wrote:
> >> in particular, what SQL command is it trying to execute when it chokes?
>
> > The function is executing:
>
> >     EXECUTE ''update pg_attribute set attnotnull = false from pg_class where
> >                   pg_attribute.attrelid = pg_class.oid and pg_class.relname = '' ||
> >                   quote_literal(table_name) ||'' and pg_attribute.attname = '' ||
> >                   quote_literal(column_name);
>
> Well, no wonder.  You have to be superuser to do that, and it's a pretty
> bad idea even then.
>
> We do have ALTER TABLE ... SET/DROP NOT NULL since 7.3, so hacking
> pg_attribute directly isn't necessary for this anymore.

So, if we replace that with:

ALTER TABLE table_name ALTER column_name DROP NOT NULL; ?

should be good to go?  still not as clean as doing the straight DROP
COLUMN, but its a fast fix ...


----
Marc G. Fournier           Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org           Yahoo!: yscrappy              ICQ: 7615664

"Marc G. Fournier" <scrappy@postgresql.org> writes:
> So, if we replace that with:
> ALTER TABLE table_name ALTER column_name DROP NOT NULL; ?
> should be good to go?  still not as clean as doing the straight DROP
> COLUMN, but its a fast fix ...

Yeah, that's what I'd do until the PostGIS guys can rethink things at a
higher level.
        regards, tom lane

One of the great annoyances of the OpenGIS spec is the requirement for 
a "geometry_columns" table, that has a list of all the spatial columns 
and a little bit of metadata on them (what type are they, what is the 
spatial reference system of their coordinates, what is their 
dimensionality).

Unfortunately, we have been reduced to "manually" maintaining this 
table through the mechanism of the "AddGeometryColumn" 
"DropGeometryColumn" functions. As you noted, we had some old scruft in 
there dating back to the pre-DROP COLUMN days. That's gone in the 
current version.

In an idea world though, we would construct the thing as a view, so 
that when you did a CREATE TABLE that included a geometry type, you 
would automatically get a row in geometry_columns. That requires a view 
on system tables though, and that just does not work. :/

Any thoughts on a nice implementation?

Paul

On Tuesday, January 20, 2004, at 09:07 AM, Tom Lane wrote:

> "Marc G. Fournier" <scrappy@postgresql.org> writes:
>> So, if we replace that with:
>> ALTER TABLE table_name ALTER column_name DROP NOT NULL; ?
>> should be good to go?  still not as clean as doing the straight DROP
>> COLUMN, but its a fast fix ...
>
> Yeah, that's what I'd do until the PostGIS guys can rethink things at a
> higher level.
>
>             regards, tom lane
>
> ---------------------------(end of 
> broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to 
> majordomo@postgresql.org
>     Paul Ramsey     Refractions Research     Email: pramsey@refractions.net     Phone: (250) 885-0632

Paul Ramsey <pramsey@refractions.net> writes:
> In an idea world though, we would construct the thing as a view, so 
> that when you did a CREATE TABLE that included a geometry type, you 
> would automatically get a row in geometry_columns. That requires a view 
> on system tables though, and that just does not work. :/

Uh, what makes you say it doesn't work?
        regards, tom lane

Bitter experience... I am going to cc Dave here, because I could swear 
we went through many conniptions trying to make this work.

And yet I just did this:

create view mytables as select relname from pg_class where relam = 0 
and relname not like 'pg_%';

And it seems to work fine.

Oh, now I remember. The deal was not views, it was triggers. Since our 
geometry_columns contains some information not available via a query on 
existing data, a trigger was what we wanted, so we could harvest the 
information from a variety of places, and have some spare columns for 
things like the geometry selectivity stats.

Paul

On Tuesday, February 3, 2004, at 11:00 AM, Tom Lane wrote:

> Paul Ramsey <pramsey@refractions.net> writes:
>> In an idea world though, we would construct the thing as a view, so
>> that when you did a CREATE TABLE that included a geometry type, you
>> would automatically get a row in geometry_columns. That requires a 
>> view
>> on system tables though, and that just does not work. :/
>
> Uh, what makes you say it doesn't work?
>
>             regards, tom lane
>     Paul Ramsey     Refractions Research     Email: pramsey@refractions.net     Phone: (250) 885-0632

Paul Ramsey <pramsey@refractions.net> writes:
> Oh, now I remember. The deal was not views, it was triggers.

Oh, okay.  You're right, we don't do triggers on system tables.  But
couldn't you combine a view on the system tables with storage of
additional data outside?
        regards, tom lane

Actually, in my wet dream, we stored everything in system tables. 
Dimensionality and SRID became parameters of the geometry, the 
selectivity stats lived in the system stats table (as Mark's patch 
should hopefully do) and the geometry_columns view just pulled 
everything together into one user-convenient location.

CREATE TABLE foo ( mygeom POLYGON(4326) );
CREATE TABLE bar ( mygeom MULTILINESTRING(20711, 2 ) );

I think we had this discussion before though, and the "parameterized" 
types, like varchar(256), were not available for extended types, like 
our geometries.

P.

On Tuesday, February 3, 2004, at 12:06 PM, Tom Lane wrote:

> Paul Ramsey <pramsey@refractions.net> writes:
>> Oh, now I remember. The deal was not views, it was triggers.
>
> Oh, okay.  You're right, we don't do triggers on system tables.  But
> couldn't you combine a view on the system tables with storage of
> additional data outside?
>
>             regards, tom lane
>     Paul Ramsey     Refractions Research     Email: pramsey@refractions.net     Phone: (250) 885-0632

Paul Ramsey <pramsey@refractions.net> writes:
> I think we had this discussion before though, and the "parameterized" 
> types, like varchar(256), were not available for extended types, like 
> our geometries.

I can't see any way to handle parameterized types without extending the
grammar individually for each one --- otherwise it's too hard to tell
them apart from function calls.  That makes it a bit hard to do 'em
as plug-ins :-(.  The grammar hacks are certainly ugly though, and if
someone could think of a way, I'm all ears...
        regards, tom lane

> I can't see any way to handle parameterized types without extending the
> grammar individually for each one --- otherwise it's too hard to tell
> them apart from function calls.  That makes it a bit hard to do 'em
> as plug-ins :-(.  The grammar hacks are certainly ugly though, and if
> someone could think of a way, I'm all ears...

Disallow it in table definitions, but allow it in domain definitions...

Chris

Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> I can't see any way to handle parameterized types without extending the
>> grammar individually for each one --- otherwise it's too hard to tell
>> them apart from function calls.

> Disallow it in table definitions, but allow it in domain definitions...

Those two cases are not hard, because in those scenarios the parser
knows it is expecting a type specification.  The real problem is this
syntax for typed literals:    typename 'string'
which occurs in ordinary expressions.  So when you see "name(" you
aren't real sure if you're seeing the start of a function call or the
start of a typed-literal construct.  And it's very hard to postpone that
decision until you see what comes after the right paren.
        regards, tom lane

> Those two cases are not hard, because in those scenarios the parser
> knows it is expecting a type specification.  The real problem is this
> syntax for typed literals:
>         typename 'string'
> which occurs in ordinary expressions.  So when you see "name(" you
> aren't real sure if you're seeing the start of a function call or the
> start of a typed-literal construct.  And it's very hard to postpone that
> decision until you see what comes after the right paren.

Just disallow that particular case for custom types :P

Will this work:  'string'::typename

Chris

Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> Those two cases are not hard, because in those scenarios the parser
>> knows it is expecting a type specification.  The real problem is this
>> syntax for typed literals:
>> typename 'string'

> Just disallow that particular case for custom types :P

Well, maybe we could --- comments?  Tom Lockhart went to some lengths to
support that, but now that he's gafiated we could perhaps rethink it.
AFAICS the SQL spec only requires this syntax for certain built-in types.
Tom wanted to generalize that to all datatypes that Postgres supports,
and that seems like a reasonable goal ... but if it prevents getting to
other reasonable goals then we ought to think twice.

> Will this work:  'string'::typename

Yes, since the :: cues the parser to expect a typename next.
        regards, tom lane

tgl wrote:
> Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> >> Those two cases are not hard, because in those scenarios the parser
> >> knows it is expecting a type specification.  The real problem is this
> >> syntax for typed literals:
> >> typename 'string'
> 
> > Just disallow that particular case for custom types :P
> 
> Well, maybe we could --- comments?  Tom Lockhart went to some lengths to
> support that, but now that he's gafiated we could perhaps rethink it.
> AFAICS the SQL spec only requires this syntax for certain built-in types.
> Tom wanted to generalize that to all datatypes that Postgres supports,
> and that seems like a reasonable goal ... but if it prevents getting to
> other reasonable goals then we ought to think twice.

If it's not for SQL conformance
I don't think we really need to generalize that.
As far as there are other means to gain the same result...

'string'::type(parameter) can be the "general" postgres version.
while varchar(2) 'string' can be the standard SQL version (not general).

--strk;

> 
> > Will this work:  'string'::typename
> 
> Yes, since the :: cues the parser to expect a typename next.
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend