Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
(2012/02/23 5:32), Alvaro Herrera wrote:
My only concern on the patch is
+static void
+AlterForeignServerOwner_internal(Relation rel, HeapTuple tup, Oid
newOwnerId)
+{
+Form_pg_foreign_server form;
-srvId = HeapTupleGetOid(tup);
form = (Form_pg_foreign_server) GETSTRUCT(tup);
if (form-srvowner != newOwnerId)
@@ -366,10 +388,15 @@ AlterForeignServerOwner(const char *name, Oid
newOwnerId)
/* Superusers can always do it */
if (!superuser())
{
I wonder if superusers can always do it. For example, is it OK for
superusers to change the ownership of a foreign server owned by old_role
to new_role that doesn't have USAGE privilege on its foreign data wrapper.
Well, permission checking are just what they were before the patch. I
did not change them here. I didn't participate in the discussions that
led to the current behavior, but as far as I know the guiding principle
here is that superusers always can do whatever they please. Maybe what
you point out is a bug in the behavior (both before and after my patch),
but if so, please raise it separately.
OK. Thanks.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
(2012/02/22 9:30), Tom Lane wrote:
Alvaro Herreraalvhe...@commandprompt.com writes:
Excerpts from Alvaro Herrera's message of mar feb 21 15:54:03 -0300 2012:
Excerpts from Tom Lane's message of lun feb 20 12:37:45 -0300 2012:
As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php
there is no switch case in shdepReassignOwned for foreign data wrappers.
I'm gonna take a stab at fixing this now (the simple way).
Here's the patch I have; last minute opinions. I intend to push a
backpatch to 9.1 and 9.0. 8.4 has the same problem, but since Heikki
punted in e356743f3ed45c36dcc4d0dbf6c1e8751b3d70b5, I'm not going to
bother either.
Looks roughly like what I expected, but I haven't tested it.
I did some tests. The results look good to me. Please find attached a
logfile. My only concern on the patch is
+static void
+AlterForeignServerOwner_internal(Relation rel, HeapTuple tup, Oid
newOwnerId)
+{
+ Form_pg_foreign_server form;
- srvId = HeapTupleGetOid(tup);
form = (Form_pg_foreign_server) GETSTRUCT(tup);
if (form-srvowner != newOwnerId)
@@ -366,10 +388,15 @@ AlterForeignServerOwner(const char *name, Oid
newOwnerId)
/* Superusers can always do it */
if (!superuser())
{
I wonder if superusers can always do it. For example, is it OK for
superusers to change the ownership of a foreign server owned by old_role
to new_role that doesn't have USAGE privilege on its foreign data wrapper.
Best regards,
Etsuro Fujita
$ psql postgres
psql (9.2devel)
Type help for help.
postgres=# CREATE ROLE reassign_fdw_user LOGIN SUPERUSER;
CREATE ROLE
postgres=# SET SESSION AUTHORIZATION 'reassign_fdw_user';
SET
postgres=# CREATE ROLE regress_test_role;
CREATE ROLE
postgres=# CREATE ROLE regress_test_role2;
CREATE ROLE
postgres=# CREATE ROLE regress_test_role_super SUPERUSER;
CREATE ROLE
postgres=# CREATE FOREIGN DATA WRAPPER foo;
CREATE FOREIGN DATA WRAPPER
postgres=# \dew
List of foreign-data wrappers
Name | Owner | Handler | Validator
--+---+-+---
foo | reassign_fdw_user | - | -
(1 row)
postgres=# REASSIGN OWNED BY reassign_fdw_user TO regress_test_role;
ERROR: permission denied to change owner of foreign-data wrapper foo
HINT: The owner of a foreign-data wrapper must be a superuser.
postgres=# REASSIGN OWNED BY reassign_fdw_user TO regress_test_role_super;
REASSIGN OWNED
postgres=# \dew
List of foreign-data wrappers
Name | Owner | Handler | Validator
--+-+-+---
foo | regress_test_role_super | - | -
(1 row)
postgres=# REASSIGN OWNED BY regress_test_role_super TO regress_test_role;
ERROR: permission denied to change owner of foreign-data wrapper foo
HINT: The owner of a foreign-data wrapper must be a superuser.
postgres=# REASSIGN OWNED BY regress_test_role_super TO reassign_fdw_user;
REASSIGN OWNED
postgres=# GRANT USAGE ON FOREIGN DATA WRAPPER foo TO regress_test_role;
GRANT
postgres=# SET ROLE regress_test_role;
SET
postgres= CREATE SERVER t1 FOREIGN DATA WRAPPER foo;
CREATE SERVER
postgres= REASSIGN OWNED by regress_test_role TO regress_test_role2;
ERROR: permission denied to reassign objects
postgres= RESET ROLE;
RESET
postgres=# REASSIGN OWNED by regress_test_role TO regress_test_role2;
REASSIGN OWNED
postgres=# \des+
List of foreign servers
Name | Owner| Foreign-data wrapper | Access privileges | Type | V
ersion | FDW Options | Description
--++--+---+--+--
---+-+-
t1 | regress_test_role2 | foo | | |
| |
(1 row)
postgres=#
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
Excerpts from Etsuro Fujita's message of mié feb 22 05:37:36 -0300 2012:
I did some tests. The results look good to me. Please find attached a
logfile.
Thanks.
My only concern on the patch is
+static void
+AlterForeignServerOwner_internal(Relation rel, HeapTuple tup, Oid
newOwnerId)
+{
+Form_pg_foreign_server form;
-srvId = HeapTupleGetOid(tup);
form = (Form_pg_foreign_server) GETSTRUCT(tup);
if (form-srvowner != newOwnerId)
@@ -366,10 +388,15 @@ AlterForeignServerOwner(const char *name, Oid
newOwnerId)
/* Superusers can always do it */
if (!superuser())
{
I wonder if superusers can always do it. For example, is it OK for
superusers to change the ownership of a foreign server owned by old_role
to new_role that doesn't have USAGE privilege on its foreign data wrapper.
Well, permission checking are just what they were before the patch. I
did not change them here. I didn't participate in the discussions that
led to the current behavior, but as far as I know the guiding principle
here is that superusers always can do whatever they please. Maybe what
you point out is a bug in the behavior (both before and after my patch),
but if so, please raise it separately.
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
Excerpts from Tom Lane's message of mar feb 21 21:30:39 -0300 2012: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Alvaro Herrera's message of mar feb 21 15:54:03 -0300 2012: Excerpts from Tom Lane's message of lun feb 20 12:37:45 -0300 2012: As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. I'm gonna take a stab at fixing this now (the simple way). Here's the patch I have; last minute opinions. I intend to push a backpatch to 9.1 and 9.0. 8.4 has the same problem, but since Heikki punted in e356743f3ed45c36dcc4d0dbf6c1e8751b3d70b5, I'm not going to bother either. Looks roughly like what I expected, but I haven't tested it. Thanks, pushed. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
Excerpts from Tom Lane's message of lun feb 20 12:37:45 -0300 2012: Robert Haas robertmh...@gmail.com writes: On Wed, Feb 15, 2012 at 12:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. The obvious short-term answer (and probably the only back-patchable one) is to add a case for that object type. But after all the refactoring that's been done in the general area of this type of command, I'm a bit surprised that shdepReassignOwned still looks like this. Can't we merge this knowledge into someplace where it doesn't have to be maintained separately? Hmm. I guess we could add function pointers to the ObjectProperty array in objectaddress.c. Then we could just search the array for the catalog ID and call the associated function through the function pointer, rather than having a switch in shdepReassignOwned(). Since anyone adding a new object type ought to be looking at objectaddress.c anyway, that would be one less place for people to forget to update. I was wondering more whether there isn't some single entry point that would allow access to ALTER OWNER functionality for any object type. If we still are in a situation where new shdepReassignOwned-specific code has to be written for every object type, it's not really much better. BTW, code freeze for the upcoming releases is Thursday ... is anyone going to actually fix this bug before then? I'm unlikely to find the time myself. I'm gonna take a stab at fixing this now (the simple way). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
Excerpts from Alvaro Herrera's message of mar feb 21 15:54:03 -0300 2012: Excerpts from Tom Lane's message of lun feb 20 12:37:45 -0300 2012: Robert Haas robertmh...@gmail.com writes: On Wed, Feb 15, 2012 at 12:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. I'm gonna take a stab at fixing this now (the simple way). Here's the patch I have; last minute opinions. I intend to push a backpatch to 9.1 and 9.0. 8.4 has the same problem, but since Heikki punted in e356743f3ed45c36dcc4d0dbf6c1e8751b3d70b5, I'm not going to bother either. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support reassign-fdw.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Alvaro Herrera's message of mar feb 21 15:54:03 -0300 2012: Excerpts from Tom Lane's message of lun feb 20 12:37:45 -0300 2012: As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. I'm gonna take a stab at fixing this now (the simple way). Here's the patch I have; last minute opinions. I intend to push a backpatch to 9.1 and 9.0. 8.4 has the same problem, but since Heikki punted in e356743f3ed45c36dcc4d0dbf6c1e8751b3d70b5, I'm not going to bother either. Looks roughly like what I expected, but I haven't tested it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
On Wed, Feb 15, 2012 at 12:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. The obvious short-term answer (and probably the only back-patchable one) is to add a case for that object type. But after all the refactoring that's been done in the general area of this type of command, I'm a bit surprised that shdepReassignOwned still looks like this. Can't we merge this knowledge into someplace where it doesn't have to be maintained separately? Hmm. I guess we could add function pointers to the ObjectProperty array in objectaddress.c. Then we could just search the array for the catalog ID and call the associated function through the function pointer, rather than having a switch in shdepReassignOwned(). Since anyone adding a new object type ought to be looking at objectaddress.c anyway, that would be one less place for people to forget to update. However, I'm not 100% sure that's an improvement. Switches by object type are probably not going to go away... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REASSIGN OWNED lacks support for FDWs
Robert Haas robertmh...@gmail.com writes: On Wed, Feb 15, 2012 at 12:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. The obvious short-term answer (and probably the only back-patchable one) is to add a case for that object type. But after all the refactoring that's been done in the general area of this type of command, I'm a bit surprised that shdepReassignOwned still looks like this. Can't we merge this knowledge into someplace where it doesn't have to be maintained separately? Hmm. I guess we could add function pointers to the ObjectProperty array in objectaddress.c. Then we could just search the array for the catalog ID and call the associated function through the function pointer, rather than having a switch in shdepReassignOwned(). Since anyone adding a new object type ought to be looking at objectaddress.c anyway, that would be one less place for people to forget to update. I was wondering more whether there isn't some single entry point that would allow access to ALTER OWNER functionality for any object type. If we still are in a situation where new shdepReassignOwned-specific code has to be written for every object type, it's not really much better. BTW, code freeze for the upcoming releases is Thursday ... is anyone going to actually fix this bug before then? I'm unlikely to find the time myself. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] REASSIGN OWNED lacks support for FDWs
As per http://archives.postgresql.org/pgsql-general/2012-02/msg00304.php there is no switch case in shdepReassignOwned for foreign data wrappers. The obvious short-term answer (and probably the only back-patchable one) is to add a case for that object type. But after all the refactoring that's been done in the general area of this type of command, I'm a bit surprised that shdepReassignOwned still looks like this. Can't we merge this knowledge into someplace where it doesn't have to be maintained separately? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
