Unix Technical Forum

A new feature patch and a bug fix

This is a discussion on A new feature patch and a bug fix within the pgsql Interfaces Pgadmin Hackers forums, part of the PostgreSQL category; --> Hi, Let's begin with the bug fix. I've found that I can set the owner of an object only ...


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > pgsql Interfaces Pgadmin Hackers

FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #1 (permalink)  
Old 04-18-2008, 09:54 AM
Guillaume Lelarge
 
Posts: n/a
Default A new feature patch and a bug fix

Hi,

Let's begin with the bug fix. I've found that I can set the owner of an
object only from login role. pgAdmin doesn't allow me to set ownership
to a group. psql allows it, so I think this is a bug. The patch named
"dlgProperty.patch" fixes this issue.

I found this when I tried to add "reassign owned by" and "drop owned by"
functionalities to pgAdmin. I first thought I would add this with the
drop-user code but I prefer to make it available alone. So I used my
well known way : a new contextual menu (in fact two : "Reassign Owned
To..." and "Drop Owned"). The first one opens a wxSingleChoiceDialog
dialog for the user to choose a role from. The second one waits for a
confirmation and then fires the SQL statement. My problem is here : on a
user contextual menu, the only connection I can get is the maintenance
connection. So I will only be able to reassign or drop objects within
the maintenance database. I was wondering if there was a way to get a
handle to the last open connection ? or if there was another way ? for
example asking the user to choose the database to connect to and fire
the SQL statement ? or asking the user to select a database where he's
already connected to ?

Thanks.

Regards.


--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com


---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #2 (permalink)  
Old 04-18-2008, 09:54 AM
Dave Page
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Guillaume Lelarge wrote:
> Hi,
>
> Let's begin with the bug fix. I've found that I can set the owner of an
> object only from login role. pgAdmin doesn't allow me to set ownership
> to a group. psql allows it, so I think this is a bug. The patch named
> "dlgProperty.patch" fixes this issue.


OK... but can you really assign ownership to a group as well as a group
role? For example, from:
http://www.postgresql.org/docs/8.0/i...ltertable.html

----
This form changes the owner of the table, index, sequence, or view to
the specified user.
----

That implies just users to me.

> I found this when I tried to add "reassign owned by" and "drop owned by"
> functionalities to pgAdmin. I first thought I would add this with the
> drop-user code but I prefer to make it available alone. So I used my
> well known way : a new contextual menu (in fact two : "Reassign Owned
> To..." and "Drop Owned"). The first one opens a wxSingleChoiceDialog
> dialog for the user to choose a role from. The second one waits for a
> confirmation and then fires the SQL statement. My problem is here : on a
> user contextual menu, the only connection I can get is the maintenance
> connection. So I will only be able to reassign or drop objects within
> the maintenance database. I was wondering if there was a way to get a
> handle to the last open connection ? or if there was another way ? for
> example asking the user to choose the database to connect to and fire
> the SQL statement ? or asking the user to select a database where he's
> already connected to ?


Yeah, I think that's going to be the only way - present a list of
databases on the server. Or do it the other way round and add the
functionality to the database object and present a list of roles to
choose from.

/D

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #3 (permalink)  
Old 04-18-2008, 09:54 AM
Guillaume Lelarge
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Dave Page wrote:
> Guillaume Lelarge wrote:
>> Let's begin with the bug fix. I've found that I can set the owner of an
>> object only from login role. pgAdmin doesn't allow me to set ownership
>> to a group. psql allows it, so I think this is a bug. The patch named
>> "dlgProperty.patch" fixes this issue.

>
> OK... but can you really assign ownership to a group as well as a group
> role? For example, from:
> http://www.postgresql.org/docs/8.0/i...ltertable.html
>
> ----
> This form changes the owner of the table, index, sequence, or view to
> the specified user.
> ----
>
> That implies just users to me.
>


I did it for a group, works great (8.1 and 8.2). Now, I don't really
know if this is really a feature or something overlooked. Either pgAdmin
is wrong either psql is.

>> I found this when I tried to add "reassign owned by" and "drop owned by"
>> functionalities to pgAdmin. I first thought I would add this with the
>> drop-user code but I prefer to make it available alone. So I used my
>> well known way : a new contextual menu (in fact two : "Reassign Owned
>> To..." and "Drop Owned"). The first one opens a wxSingleChoiceDialog
>> dialog for the user to choose a role from. The second one waits for a
>> confirmation and then fires the SQL statement. My problem is here : on a
>> user contextual menu, the only connection I can get is the maintenance
>> connection. So I will only be able to reassign or drop objects within
>> the maintenance database. I was wondering if there was a way to get a
>> handle to the last open connection ? or if there was another way ? for
>> example asking the user to choose the database to connect to and fire
>> the SQL statement ? or asking the user to select a database where he's
>> already connected to ?

>
> Yeah, I think that's going to be the only way - present a list of
> databases on the server. Or do it the other way round and add the
> functionality to the database object and present a list of roles to
> choose from.
>


I'll take a look at the query tool to see how to do this ("present a
list of databases on the server"). Another call to wxSingleChoiceDialod
seems a bad way to me... I'll need to add a new dialog, don't you think ?


--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #4 (permalink)  
Old 04-18-2008, 09:54 AM
Dave Page
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Guillaume Lelarge wrote:
> I did it for a group, works great (8.1 and 8.2). Now, I don't really
> know if this is really a feature or something overlooked. Either pgAdmin
> is wrong either psql is.


No, in 8.1+ it works for roles whether they are what we would call login
roles or group roles. In 8.0 and below we had separate users and groups,
but I'm not sure you could assign ownership to a group back then.

> I'll take a look at the query tool to see how to do this ("present a
> list of databases on the server"). Another call to wxSingleChoiceDialod
> seems a bad way to me... I'll need to add a new dialog, don't you think ?


Yes, I don't think it should be two dialogues.

/D


---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #5 (permalink)  
Old 04-18-2008, 09:54 AM
Guillaume Lelarge
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Dave Page wrote:
> Guillaume Lelarge wrote:
>> I did it for a group, works great (8.1 and 8.2). Now, I don't really
>> know if this is really a feature or something overlooked. Either pgAdmin
>> is wrong either psql is.

>
> No, in 8.1+ it works for roles whether they are what we would call login
> roles or group roles. In 8.0 and below we had separate users and groups,
> but I'm not sure you could assign ownership to a group back then.
>


You're right. I checked this on a 7.4 (I don't have a 8.0 release
available) and, with this release, a group can't be the owner of a table.

Do you think I should raise the issue (group owner on 8.1+) on
pgsql-hackers ?

>> I'll take a look at the query tool to see how to do this ("present a
>> list of databases on the server"). Another call to wxSingleChoiceDialod
>> seems a bad way to me... I'll need to add a new dialog, don't you think ?

>
> Yes, I don't think it should be two dialogues.
>


I'm working on it now.


--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #6 (permalink)  
Old 04-18-2008, 09:54 AM
Dave Page
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Guillaume Lelarge wrote:
> Dave Page wrote:
>> Guillaume Lelarge wrote:
>>> I did it for a group, works great (8.1 and 8.2). Now, I don't really
>>> know if this is really a feature or something overlooked. Either pgAdmin
>>> is wrong either psql is.

>> No, in 8.1+ it works for roles whether they are what we would call login
>> roles or group roles. In 8.0 and below we had separate users and groups,
>> but I'm not sure you could assign ownership to a group back then.
>>

>
> You're right. I checked this on a 7.4 (I don't have a 8.0 release
> available) and, with this release, a group can't be the owner of a table.
>
> Do you think I should raise the issue (group owner on 8.1+) on
> pgsql-hackers ?


No - I think it just works because pg_group is a view over pg_role (or
whatever it's called ) on 8.1. I would try something like:

Index: pgadmin/dlg/dlgProperty.cpp
================================================== =================
--- pgadmin/dlg/dlgProperty.cpp (révision 6910)
+++ pgadmin/dlg/dlgProperty.cpp (copie de travail)
@@ -432,7 +432,14 @@

void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
{
- FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"),
cb1, cb2);
+ if (connection->BackendMinimumVersion(8, 1))
+ {
+ FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"),
cb1, cb2);
+ }
+ else
+ {
+ FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY 1"),
cb1, cb2);
+ }
}


>>> I'll take a look at the query tool to see how to do this ("present a
>>> list of databases on the server"). Another call to wxSingleChoiceDialod
>>> seems a bad way to me... I'll need to add a new dialog, don't you think ?

>> Yes, I don't think it should be two dialogues.
>>

>
> I'm working on it now.


'K.

/D


---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #7 (permalink)  
Old 04-18-2008, 09:54 AM
Guillaume Lelarge
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Dave Page wrote:
> Guillaume Lelarge wrote:
>> Dave Page wrote:
>>> Guillaume Lelarge wrote:
>>>> I did it for a group, works great (8.1 and 8.2). Now, I don't really
>>>> know if this is really a feature or something overlooked. Either pgAdmin
>>>> is wrong either psql is.
>>> No, in 8.1+ it works for roles whether they are what we would call login
>>> roles or group roles. In 8.0 and below we had separate users and groups,
>>> but I'm not sure you could assign ownership to a group back then.
>>>

>> You're right. I checked this on a 7.4 (I don't have a 8.0 release
>> available) and, with this release, a group can't be the owner of a table.
>>
>> Do you think I should raise the issue (group owner on 8.1+) on
>> pgsql-hackers ?

>
> No - I think it just works because pg_group is a view over pg_role (or
> whatever it's called ) on 8.1. I would try something like:
>
> Index: pgadmin/dlg/dlgProperty.cpp
> ================================================== =================
> --- pgadmin/dlg/dlgProperty.cpp (révision 6910)
> +++ pgadmin/dlg/dlgProperty.cpp (copie de travail)
> @@ -432,7 +432,14 @@
>
> void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
> {
> - FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"),
> cb1, cb2);
> + if (connection->BackendMinimumVersion(8, 1))
> + {
> + FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"),
> cb1, cb2);
> + }
> + else
> + {
> + FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY 1"),
> cb1, cb2);
> + }
> }
>


I'm OK with this.

Thanks.


--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #8 (permalink)  
Old 04-18-2008, 09:54 AM
Dave Page
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Guillaume Lelarge wrote:
> Dave Page wrote:
>> Index: pgadmin/dlg/dlgProperty.cpp
>> ================================================== =================
>> --- pgadmin/dlg/dlgProperty.cpp (révision 6910)
>> +++ pgadmin/dlg/dlgProperty.cpp (copie de travail)
>> @@ -432,7 +432,14 @@
>>
>> void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
>> {
>> - FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"),
>> cb1, cb2);
>> + if (connection->BackendMinimumVersion(8, 1))
>> + {
>> + FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"),
>> cb1, cb2);
>> + }
>> + else
>> + {
>> + FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY 1"),
>> cb1, cb2);
>> + }
>> }
>>

>
> I'm OK with this.


Please go ahead and commit at your leisure :-)

/D

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #9 (permalink)  
Old 04-18-2008, 09:54 AM
Guillaume Lelarge
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Dave Page wrote:
> Guillaume Lelarge wrote:
> [...]
>>>> I'll take a look at the query tool to see how to do this ("present a
>>>> list of databases on the server"). Another call to wxSingleChoiceDialod
>>>> seems a bad way to me... I'll need to add a new dialog, don't you think ?
>>> Yes, I don't think it should be two dialogues.
>>>

>> I'm working on it now.

>
> 'K.
>


I've finally done it. It's the first time I create a new dialog, so it
probably need some tweaks. I attached the patch and a tar.gz file for
the new files.

Regards.


--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #10 (permalink)  
Old 04-18-2008, 09:55 AM
Dave Page
 
Posts: n/a
Default Re: A new feature patch and a bug fix

Hi Guillaume,

On 22/12/2007, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> I've finally done it. It's the first time I create a new dialog, so it
> probably need some tweaks. I attached the patch and a tar.gz file for
> the new files.


Finally, some feedback for you!!

- In pgRole::ReassignDropOwnedTo, objets should be objects.

- In pgRole::ReassignDropOwnedTo an error should be raised if the
connection cannot be opened.

- The database selection in dlgReassignDropOwned should respect the DB
restriction setting for the server.

- Remember to append a 'd' to all size/position values in XRC files to
denote cross-platform friendly dialog units.

- The dialog needs some work. Get yourself a copy of XRCed - it will
help enormously. Try to keep the layout and sizing in line with other
dialogues. Use sizers to keep everything in line rather than absolute
positioning (new dialogues do this, older ones might not). It's a bit
fiddly until you get the hang of them, but it does make things much
easier to get right across platforms.

Thanks, Dave

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 02:27 AM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
www.UnixAdminTalk.com