[pgadmin-hackers] pgAdmin III commit: Tag REL-1_14_0-BETA3 has been created.

[Dave Page]

Tag REL-1_14_0-BETA3 has been created.
View: 
http://git.postgresql.org/gitweb?p=pgadmin3.git;a=tag;h=refs/tags/REL-1_14_0-BETA3

Log Message
---
Tag beta 3
-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] Overhaul of type attributes modification

[Thom Brown]

Hi all,

I noticed that if you add, delete, rename or change the type,
collation or precision of a composite type attribute, it deletes all
of them then adds them all back in.  Obviously attribute additions,
deletions and modifications may only occur for types under PostgreSQL
9.1, but I thought it a bit extreme to actually drop all types if
you're adding a new one in, or just removing one.  Also, if you rename
a type, it drops all attributes again then re-adds them, just to have
an attribute with a different name.

So I've attached a patch which "does the right thing".  Take the
example of the following type:

CREATE TYPE bark AS
   (one text,
two text,
three text,
four text,
five text);

Say we wanted to remove "two", change the type of three to uuid,
rename four to forty and add an extra text type of six.

Normally we'd just get:

ALTER TYPE bark DROP ATTRIBUTE one;
ALTER TYPE bark DROP ATTRIBUTE two;
ALTER TYPE bark DROP ATTRIBUTE three;
ALTER TYPE bark DROP ATTRIBUTE four;
ALTER TYPE bark DROP ATTRIBUTE five;
ALTER TYPE bark ADD ATTRIBUTE one text;
ALTER TYPE bark ADD ATTRIBUTE three text;
ALTER TYPE bark ADD ATTRIBUTE forty uuid;
ALTER TYPE bark ADD ATTRIBUTE five text;
ALTER TYPE bark ADD ATTRIBUTE six uuid;

With these changes we'd now get:

ALTER TYPE bark DROP ATTRIBUTE two;
ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
ALTER TYPE bark ADD ATTRIBUTE six text;

.. except now those are also nicely indented to be more readable for
types with long schemas/type names.

e.g.
ALTER TYPE long_schema_name.quite_a_long_table_name
  ADD ATTRIBUTE "suspiciously long attribute name"?

It also fixes a bug whereby if you have a precision specified, the
word COLLATE mysteriously appears after the type whether or not you
have a collation assigned because the collation condition was based on
the precision being present for some reason.  And if you actually
assigned a collation for a valid data type, it wouldn't appear at all,
so that's fixed too.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/pgadmin/dlg/dlgType.cpp b/pgadmin/dlg/dlgType.cpp
index 5f3a36c..b951d7b 100644
--- a/pgadmin/dlg/dlgType.cpp
+++ b/pgadmin/dlg/dlgType.cpp
@@ -285,6 +285,7 @@ int dlgType::Go(bool modal)
 			memberLengths.Add(typeLength);
 			memberPrecisions.Add(typePrecision);
 			memberCollations.Add(elements.Item(i + 2));
+			memberOriginalNames.Add(elements.Item(i));
 		}
 
 		cbDatatype->Enable(changeok);
@@ -533,6 +534,7 @@ void dlgType::OnMemberAdd(wxCommandEvent &ev)
 		memberLengths.Add(length);
 		memberPrecisions.Add(precision);
 		memberCollations.Add(collation);
+		memberOriginalNames.Add(wxEmptyString);
 	}
 
 	CheckChange();
@@ -574,10 +576,12 @@ void dlgType::OnMemberChange(wxCommandEvent &ev)
 			memberLengths.Insert(length, pos);
 			memberPrecisions.Insert(precision, pos);
 			memberCollations.Insert(collation, pos);
+//			memberOriginalNames.Insert(name, pos);
 			memberTypes.RemoveAt(pos + 1);
 			memberLengths.RemoveAt(pos + 1);
 			memberPrecisions.RemoveAt(pos + 1);
 			memberCollations.RemoveAt(pos + 1);
+//			memberOriginalNames.RemoveAt(pos + 1);
 		}
 	}
 
@@ -596,6 +600,7 @@ void dlgType::OnMemberRemove(wxCommandEvent &ev)
 		memberLengths.RemoveAt(pos);
 		memberPrecisions.RemoveAt(pos);
 		memberCollations.RemoveAt(pos);
+		memberOriginalNames.RemoveAt(pos);
 	}
 	CheckChange();
 }
@@ -833,7 +838,7 @@ wxString dlgType::GetFullTypeName(int type)
 			typname += wxT(",") + memberPrecisions.Item(type);
 		typname += wxT(")");
 	}
-	if (!memberPrecisions.Item(type).IsEmpty() && memberPrecisions.Item(type) != wxT("pg_catalog.\"default\""))
+	if (!memberCollations.Item(type).IsEmpty() && memberCollations.Item(type) != wxT("pg_catalog.\"default\""))
 		typname += wxT(" COLLATE ") + memberCollations.Item(type);
 
 	return typname;
@@ -842,46 +847,78 @@ wxString dlgType::GetFullTypeName(int type)
 wxString dlgType::GetSqlForTypes()
 {
 	wxString sql = wxEmptyString;
-	wxString old_name, old_type, old_collation, new_name, new_type, new_collation;
+	wxString objname, old_name, old_type, old_collation, new_name, new_type, new_collation, original_name;
 	wxArrayString elements = type->GetTypesArray();
-	bool modified = lstMembers->GetItemCount() * 3 != (int)elements.GetCount();
-	size_t i;
+	size_t newindex;
+	size_t oldindex = 0;
+	inthold = 0;
+	objname = schema->GetQuotedPrefix() + qtIdent(GetName());
 
-	// Check if there is a change
-	for (int i = 0 ; i < lstMembers->GetItemCount() && !modified; i++)
+	for (newindex = 0 ; newindex < lstMembers->GetItemCount() ; newindex = newindex + 1 - hold)
 	{
-		old_name = elements.Item(i * 3);
-		old_type = elements.Item(i * 3 + 1);
-		old_collation = elements.Item(i * 3 + 2);
-		new_name = lstMembers->GetItemText(i);
-		new_type = GetFullTypeName(i);
-		new_collation = memberCollations.Item(i);
-		modified = modified || 

Re: [pgadmin-hackers] Overhaul of type attributes modification

[Thom Brown]

On 8 July 2011 15:19, Thom Brown  wrote:
> Hi all,
>
> I noticed that if you add, delete, rename or change the type,
> collation or precision of a composite type attribute, it deletes all
> of them then adds them all back in.  Obviously attribute additions,
> deletions and modifications may only occur for types under PostgreSQL
> 9.1, but I thought it a bit extreme to actually drop all types if
> you're adding a new one in, or just removing one.  Also, if you rename
> a type, it drops all attributes again then re-adds them, just to have
> an attribute with a different name.
>
> So I've attached a patch which "does the right thing".  Take the
> example of the following type:
>
> CREATE TYPE bark AS
>   (one text,
>    two text,
>    three text,
>    four text,
>    five text);
>
> Say we wanted to remove "two", change the type of three to uuid,
> rename four to forty and add an extra text type of six.
>
> Normally we'd just get:
>
> ALTER TYPE bark DROP ATTRIBUTE one;
> ALTER TYPE bark DROP ATTRIBUTE two;
> ALTER TYPE bark DROP ATTRIBUTE three;
> ALTER TYPE bark DROP ATTRIBUTE four;
> ALTER TYPE bark DROP ATTRIBUTE five;
> ALTER TYPE bark ADD ATTRIBUTE one text;
> ALTER TYPE bark ADD ATTRIBUTE three text;
> ALTER TYPE bark ADD ATTRIBUTE forty uuid;
> ALTER TYPE bark ADD ATTRIBUTE five text;
> ALTER TYPE bark ADD ATTRIBUTE six uuid;
>
> With these changes we'd now get:
>
> ALTER TYPE bark DROP ATTRIBUTE two;
> ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
> ALTER TYPE bark ADD ATTRIBUTE six text;
>
> .. except now those are also nicely indented to be more readable for
> types with long schemas/type names.
>
> e.g.
> ALTER TYPE long_schema_name.quite_a_long_table_name
>  ADD ATTRIBUTE "suspiciously long attribute name"?
>
> It also fixes a bug whereby if you have a precision specified, the
> word COLLATE mysteriously appears after the type whether or not you
> have a collation assigned because the collation condition was based on
> the precision being present for some reason.  And if you actually
> assigned a collation for a valid data type, it wouldn't appear at all,
> so that's fixed too.

Actually I noticed that the example I gave of what would occur before
the patch highlights another bug.  Notice that forty and six have the
type of uuid, and three doesn't?  Three should have been the one with
uuid, and forty and sixty should have been text.  That's not my typo
as that's come from PgAdmin 1.14 beta 2.  So it was actually assigning
the wrong data types in those case too.  So the patch fixes those
problems too.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] Overhaul of type attributes modification

[Thom Brown]

On 8 July 2011 15:38, Thom Brown  wrote:
> On 8 July 2011 15:19, Thom Brown  wrote:
>> Hi all,
>>
>> I noticed that if you add, delete, rename or change the type,
>> collation or precision of a composite type attribute, it deletes all
>> of them then adds them all back in.  Obviously attribute additions,
>> deletions and modifications may only occur for types under PostgreSQL
>> 9.1, but I thought it a bit extreme to actually drop all types if
>> you're adding a new one in, or just removing one.  Also, if you rename
>> a type, it drops all attributes again then re-adds them, just to have
>> an attribute with a different name.
>>
>> So I've attached a patch which "does the right thing".  Take the
>> example of the following type:
>>
>> CREATE TYPE bark AS
>>   (one text,
>>    two text,
>>    three text,
>>    four text,
>>    five text);
>>
>> Say we wanted to remove "two", change the type of three to uuid,
>> rename four to forty and add an extra text type of six.
>>
>> Normally we'd just get:
>>
>> ALTER TYPE bark DROP ATTRIBUTE one;
>> ALTER TYPE bark DROP ATTRIBUTE two;
>> ALTER TYPE bark DROP ATTRIBUTE three;
>> ALTER TYPE bark DROP ATTRIBUTE four;
>> ALTER TYPE bark DROP ATTRIBUTE five;
>> ALTER TYPE bark ADD ATTRIBUTE one text;
>> ALTER TYPE bark ADD ATTRIBUTE three text;
>> ALTER TYPE bark ADD ATTRIBUTE forty uuid;
>> ALTER TYPE bark ADD ATTRIBUTE five text;
>> ALTER TYPE bark ADD ATTRIBUTE six uuid;
>>
>> With these changes we'd now get:
>>
>> ALTER TYPE bark DROP ATTRIBUTE two;
>> ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
>> ALTER TYPE bark ADD ATTRIBUTE six text;
>>
>> .. except now those are also nicely indented to be more readable for
>> types with long schemas/type names.
>>
>> e.g.
>> ALTER TYPE long_schema_name.quite_a_long_table_name
>>  ADD ATTRIBUTE "suspiciously long attribute name"?
>>
>> It also fixes a bug whereby if you have a precision specified, the
>> word COLLATE mysteriously appears after the type whether or not you
>> have a collation assigned because the collation condition was based on
>> the precision being present for some reason.  And if you actually
>> assigned a collation for a valid data type, it wouldn't appear at all,
>> so that's fixed too.
>
> Actually I noticed that the example I gave of what would occur before
> the patch highlights another bug.  Notice that forty and six have the
> type of uuid, and three doesn't?  Three should have been the one with
> uuid, and forty and sixty should have been text.  That's not my typo
> as that's come from PgAdmin 1.14 beta 2.  So it was actually assigning
> the wrong data types in those case too.  So the patch fixes those
> problems too.

Also I noticed that my example of the new output doesn't show the
three datatype being changed to uuid.  This was just me forgetting to
click the Change button when I changed the type, so it would actually
come out as:

ALTER TYPE bark DROP ATTRIBUTE two;
ALTER TYPE bark DROP ATTRIBUTE three;
ALTER TYPE bark ADD ATTRIBUTE three uuid;
ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
ALTER TYPE bark ADD ATTRIBUTE six text;

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Guillaume Lelarge]

On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
> On 7 July 2011 23:20, Guillaume Lelarge  wrote:
> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
> >> add_schemas_to_all_items_filtered.patch - same patch as before, but
> >> with the above patch contents removed or adjusted to assume the fixes
> >> have been applied.
> >>
> >
> > Not yet done. I'll try to take care of this tomorrow.
> 
> Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
> in I shouldn't have, and that I don't break anything for previous
> versions.
> 

OK, just a few things that I fixed on your patch:
 * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
   previous patches, I don't keep that.
 * all extension changes are wrong according to me because they aren't
   schema objects, but database objects. I don't keep them.
 * you added possibility to change a type, and a sequence, but forgot to
   add the code to detect the changes in CheckChange. I fixed that
 * in dlgForeignTable.cpp, you declare a direction variable, but don't
   use it. Fixed that too.
 * still in dlgForeignTable.cpp, you enable the schema widget only when
   the user is connected in 9.1 but this guy can have this window only
   when he's on 9.1. Fixed that.

And that should be all. The fixed patch is attached. There is one
remaining issue: how to refresh the object's parent node in the new
schema?


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


add_schemas_v2.patch.bz2
Description: application/bzip

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Thom Brown]

On 8 July 2011 19:46, Guillaume Lelarge  wrote:
> On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
>> On 7 July 2011 23:20, Guillaume Lelarge  wrote:
>> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
>> >> add_schemas_to_all_items_filtered.patch - same patch as before, but
>> >> with the above patch contents removed or adjusted to assume the fixes
>> >> have been applied.
>> >>
>> >
>> > Not yet done. I'll try to take care of this tomorrow.
>>
>> Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
>> in I shouldn't have, and that I don't break anything for previous
>> versions.
>>
>
> OK, just a few things that I fixed on your patch:
>  * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
>   previous patches, I don't keep that.

Good spot.

>  * all extension changes are wrong according to me because they aren't
>   schema objects, but database objects. I don't keep them.

The reason why I based it on schema is because I wanted it to inherit
the schema combobox object and its source for a list of schemas so
lots of redundant code could be removed.  There should be no
functional difference, but I'm probably missing the point here. :)

>  * you added possibility to change a type, and a sequence, but forgot to
>   add the code to detect the changes in CheckChange. I fixed that

Good spot for sequence, but looking at the patch, the check is in
there for dlgType.

>  * in dlgForeignTable.cpp, you declare a direction variable, but don't
>   use it. Fixed that too.

Hehe, that wasn't mine.  It was in there already. :)

>  * still in dlgForeignTable.cpp, you enable the schema widget only when
>   the user is connected in 9.1 but this guy can have this window only
>   when he's on 9.1. Fixed that.

Yes, that check didn't need to be in there.

> And that should be all. The fixed patch is attached. There is one
> remaining issue: how to refresh the object's parent node in the new
> schema?

I played around with rebuilding the node path with the new schema name
to refresh it, but I kept getting endless data type issues, and have
no idea how this node stuff works, so I'm not sure what to do about
that.  Have any guidance?  If not, I might have another attempt at
working out how to manipulate nodes.

Thanks for spending time on this :)

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Dave Page]

>>  * all extension changes are wrong according to me because they aren't
>>   schema objects, but database objects. I don't keep them.
>
> The reason why I based it on schema is because I wanted it to inherit
> the schema combobox object and its source for a list of schemas so
> lots of redundant code could be removed.  There should be no
> functional difference, but I'm probably missing the point here. :)

It's a misuse of the class, because it's intended to represent the
schema the object is in, not one it's related to in some other way.
We've made the mistake of trying to use these classes in ways that
weren't intended in the past, and it's bitten us badly. I'm not keen
to repeat that, for the sake of a few lines of code to store a schema
name,

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Thom Brown]

On 8 July 2011 20:15, Dave Page  wrote:
>>>  * all extension changes are wrong according to me because they aren't
>>>   schema objects, but database objects. I don't keep them.
>>
>> The reason why I based it on schema is because I wanted it to inherit
>> the schema combobox object and its source for a list of schemas so
>> lots of redundant code could be removed.  There should be no
>> functional difference, but I'm probably missing the point here. :)
>
> It's a misuse of the class, because it's intended to represent the
> schema the object is in, not one it's related to in some other way.
> We've made the mistake of trying to use these classes in ways that
> weren't intended in the past, and it's bitten us badly. I'm not keen
> to repeat that, for the sake of a few lines of code to store a schema
> name,

Whilst I don't see the actual impact, I'll concede the point since
I've only just started looking at code really so wouldn't have seen
the issue first-hand.  Extensions are a weird case for me since you
can assign them to a schema, but they are immune to being hidden by
the search path.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Dave Page]

On Friday, July 8, 2011, Thom Brown  wrote:
> On 8 July 2011 20:15, Dave Page  wrote:
  * all extension changes are wrong according to me because they aren't
   schema objects, but database objects. I don't keep them.
>>>
>>> The reason why I based it on schema is because I wanted it to inherit
>>> the schema combobox object and its source for a list of schemas so
>>> lots of redundant code could be removed.  There should be no
>>> functional difference, but I'm probably missing the point here. :)
>>
>> It's a misuse of the class, because it's intended to represent the
>> schema the object is in, not one it's related to in some other way.
>> We've made the mistake of trying to use these classes in ways that
>> weren't intended in the past, and it's bitten us badly. I'm not keen
>> to repeat that, for the sake of a few lines of code to store a schema
>> name,
>
> Whilst I don't see the actual impact, I'll concede the point since
> I've only just started looking at code really so wouldn't have seen
> the issue first-hand.  Extensions are a weird case for me since you
> can assign them to a schema, but they are immune to being hidden by
> the search path.

Think of an extension as 2 things - the packaging, and the contents.
The packaging is what we show under the Database node - it doesn't
have a schema. The contents do have a schema, and the packaging
mechanism gives you a simple way to change it.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] pgAdmin III commit: Beautify SQL code for comments and owners

[Guillaume Lelarge]

Beautify SQL code for comments and owners

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=d2a8c8ea0f98de4fec9ec6ee4e1a9289f43b542a
Author: Thom Brown 

Modified Files
--
pgadmin/dlg/dlgProperty.cpp |2 +-
pgadmin/schema/pgObject.cpp |4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] pgAdmin III commit: Beautify SQL code for comments and owners

[Guillaume Lelarge]

Beautify SQL code for comments and owners

Branch
--
REL-1_14_0_PATCHES

Details
---
http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=514d8f56f02910ff67c281eb5d98ebe5d6a15ce7
Author: Thom Brown 

Modified Files
--
pgadmin/dlg/dlgProperty.cpp |2 +-
pgadmin/schema/pgObject.cpp |4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Guillaume Lelarge]

On Fri, 2011-07-08 at 20:46 +0200, Guillaume Lelarge wrote:
> On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
> > On 7 July 2011 23:20, Guillaume Lelarge  wrote:
> > > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
> > >> add_schemas_to_all_items_filtered.patch - same patch as before, but
> > >> with the above patch contents removed or adjusted to assume the fixes
> > >> have been applied.
> > >>
> > >
> > > Not yet done. I'll try to take care of this tomorrow.
> > 
> > Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
> > in I shouldn't have, and that I don't break anything for previous
> > versions.
> > 
> 
> OK, just a few things that I fixed on your patch:
>  * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
>previous patches, I don't keep that.
>  * all extension changes are wrong according to me because they aren't
>schema objects, but database objects. I don't keep them.
>  * you added possibility to change a type, and a sequence, but forgot to
>add the code to detect the changes in CheckChange. I fixed that
>  * in dlgForeignTable.cpp, you declare a direction variable, but don't
>use it. Fixed that too.
>  * still in dlgForeignTable.cpp, you enable the schema widget only when
>the user is connected in 9.1 but this guy can have this window only
>when he's on 9.1. Fixed that.
> 

I forgot one thing:

 * extract the SQL changes to commit them now in 1.14 and master.

> And that should be all. The fixed patch is attached. There is one
> remaining issue: how to refresh the object's parent node in the new
> schema?
> 
> 


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Thom Brown]

On 8 July 2011 20:52, Guillaume Lelarge  wrote:
> On Fri, 2011-07-08 at 20:46 +0200, Guillaume Lelarge wrote:
>> On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
>> > On 7 July 2011 23:20, Guillaume Lelarge  wrote:
>> > > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
>> > >> add_schemas_to_all_items_filtered.patch - same patch as before, but
>> > >> with the above patch contents removed or adjusted to assume the fixes
>> > >> have been applied.
>> > >>
>> > >
>> > > Not yet done. I'll try to take care of this tomorrow.
>> >
>> > Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
>> > in I shouldn't have, and that I don't break anything for previous
>> > versions.
>> >
>>
>> OK, just a few things that I fixed on your patch:
>>  * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
>>    previous patches, I don't keep that.
>>  * all extension changes are wrong according to me because they aren't
>>    schema objects, but database objects. I don't keep them.
>>  * you added possibility to change a type, and a sequence, but forgot to
>>    add the code to detect the changes in CheckChange. I fixed that
>>  * in dlgForeignTable.cpp, you declare a direction variable, but don't
>>    use it. Fixed that too.
>>  * still in dlgForeignTable.cpp, you enable the schema widget only when
>>    the user is connected in 9.1 but this guy can have this window only
>>    when he's on 9.1. Fixed that.
>>
>
> I forgot one thing:
>
>  * extract the SQL changes to commit them now in 1.14 and master.
>
>> And that should be all. The fixed patch is attached. There is one
>> remaining issue: how to refresh the object's parent node in the new
>> schema?

Yes, I think I need to be more disciplined in what I include in
patches.  I got a bit carried away with this one in particular.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Guillaume Lelarge]

On Fri, 2011-07-08 at 20:06 +0100, Thom Brown wrote:
> On 8 July 2011 19:46, Guillaume Lelarge  wrote:
> > On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
> >> On 7 July 2011 23:20, Guillaume Lelarge  wrote:
> >> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
> >> >> add_schemas_to_all_items_filtered.patch - same patch as before, but
> >> >> with the above patch contents removed or adjusted to assume the fixes
> >> >> have been applied.
> >> >>
> >> >
> >> > Not yet done. I'll try to take care of this tomorrow.
> >>
> >> Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
> >> in I shouldn't have, and that I don't break anything for previous
> >> versions.
> >>
> >
> > OK, just a few things that I fixed on your patch:
> >  * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
> >   previous patches, I don't keep that.
> 
> Good spot.
> 
> >  * all extension changes are wrong according to me because they aren't
> >   schema objects, but database objects. I don't keep them.
> 
> The reason why I based it on schema is because I wanted it to inherit
> the schema combobox object and its source for a list of schemas so
> lots of redundant code could be removed.  There should be no
> functional difference, but I'm probably missing the point here. :)
> 
> >  * you added possibility to change a type, and a sequence, but forgot to
> >   add the code to detect the changes in CheckChange. I fixed that
> 
> Good spot for sequence, but looking at the patch, the check is in
> there for dlgType.
> 

I should have forgotten to "git diff" once more.

> >  * in dlgForeignTable.cpp, you declare a direction variable, but don't
> >   use it. Fixed that too.
> 
> Hehe, that wasn't mine.  It was in there already. :)
> 

Oops, sorry :)

> >  * still in dlgForeignTable.cpp, you enable the schema widget only when
> >   the user is connected in 9.1 but this guy can have this window only
> >   when he's on 9.1. Fixed that.
> 
> Yes, that check didn't need to be in there.
> 
> > And that should be all. The fixed patch is attached. There is one
> > remaining issue: how to refresh the object's parent node in the new
> > schema?
> 
> I played around with rebuilding the node path with the new schema name
> to refresh it, but I kept getting endless data type issues, and have
> no idea how this node stuff works, so I'm not sure what to do about
> that.  Have any guidance?  If not, I might have another attempt at
> working out how to manipulate nodes.
> 

I'll try to look into this, but I don't think I'll be lucky.


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Guillaume Lelarge]

On Fri, 2011-07-08 at 20:53 +0100, Thom Brown wrote:
> On 8 July 2011 20:52, Guillaume Lelarge  wrote:
> > On Fri, 2011-07-08 at 20:46 +0200, Guillaume Lelarge wrote:
> >> On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
> >> > On 7 July 2011 23:20, Guillaume Lelarge  wrote:
> >> > > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
> >> > >> add_schemas_to_all_items_filtered.patch - same patch as before, but
> >> > >> with the above patch contents removed or adjusted to assume the fixes
> >> > >> have been applied.
> >> > >>
> >> > >
> >> > > Not yet done. I'll try to take care of this tomorrow.
> >> >
> >> > Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
> >> > in I shouldn't have, and that I don't break anything for previous
> >> > versions.
> >> >
> >>
> >> OK, just a few things that I fixed on your patch:
> >>  * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
> >>previous patches, I don't keep that.
> >>  * all extension changes are wrong according to me because they aren't
> >>schema objects, but database objects. I don't keep them.
> >>  * you added possibility to change a type, and a sequence, but forgot to
> >>add the code to detect the changes in CheckChange. I fixed that
> >>  * in dlgForeignTable.cpp, you declare a direction variable, but don't
> >>use it. Fixed that too.
> >>  * still in dlgForeignTable.cpp, you enable the schema widget only when
> >>the user is connected in 9.1 but this guy can have this window only
> >>when he's on 9.1. Fixed that.
> >>
> >
> > I forgot one thing:
> >
> >  * extract the SQL changes to commit them now in 1.14 and master.
> >
> >> And that should be all. The fixed patch is attached. There is one
> >> remaining issue: how to refresh the object's parent node in the new
> >> schema?
> 
> Yes, I think I need to be more disciplined in what I include in
> patches.  I got a bit carried away with this one in particular.
> 

It's not easy to do, I understand you got carried away. Happened
sometimes to me too. Still happens actually.


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] pgAdmin III commit: Beautify SQL code for comments and owners

[Dave Page]

This isn't a bug fix, so really shouldn't be applied during beta.

On Friday, July 8, 2011, Guillaume Lelarge  wrote:
> Beautify SQL code for comments and owners
>
> Branch
> --
> REL-1_14_0_PATCHES
>
> Details
> ---
> 
http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=514d8f56f02910ff67c281eb5d98ebe5d6a15ce7
> Author: Thom Brown 
>
> Modified Files
> --
> pgadmin/dlg/dlgProperty.cpp |    2 +-
> pgadmin/schema/pgObject.cpp |    4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Guillaume Lelarge]

On Fri, 2011-07-08 at 21:55 +0200, Guillaume Lelarge wrote:
> On Fri, 2011-07-08 at 20:06 +0100, Thom Brown wrote:
> > On 8 July 2011 19:46, Guillaume Lelarge  wrote:
> > > On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
> > >> On 7 July 2011 23:20, Guillaume Lelarge  wrote:
> > >> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
> > [...]
> > > And that should be all. The fixed patch is attached. There is one
> > > remaining issue: how to refresh the object's parent node in the new
> > > schema?
> > 
> > I played around with rebuilding the node path with the new schema name
> > to refresh it, but I kept getting endless data type issues, and have
> > no idea how this node stuff works, so I'm not sure what to do about
> > that.  Have any guidance?  If not, I might have another attempt at
> > working out how to manipulate nodes.
> > 
> 
> I'll try to look into this, but I don't think I'll be lucky.
> 

I was wrong. I have it working. Code is ugly right now, so it'll need
some cleanup/refactoring/etc. Should be able to commit it tomorrow :)


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Thom Brown]

On 8 July 2011 20:57, Guillaume Lelarge  wrote:
> It's not easy to do, I understand you got carried away. Happened
> sometimes to me too. Still happens actually.

Found out this patch doesn't contain my removal of a duplicate section
of code in dlgTextSearchConfiguration.cpp, so now if the name of a
text search configuration is changed, it appears in the SQL output
twice.  My original patch moved a whole section of code, but now the
removal of the original copy is missing.  Everything else seems to
work fine though.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] pgAdmin III commit: Beautify SQL code for comments and owners

[Guillaume Lelarge]

On Fri, 2011-07-08 at 21:11 +0100, Dave Page wrote:
> This isn't a bug fix, so really shouldn't be applied during beta.
> 

Well, it's not a new feature, it's not a bugfix, it's in-between.
Strictly speaking, you're probably right. I can revert the patch if you
want, it's no big deal. But I mean, I just added three \n.


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Guillaume Lelarge]

On Sat, 2011-07-09 at 00:12 +0100, Thom Brown wrote:
> On 8 July 2011 20:57, Guillaume Lelarge  wrote:
> > It's not easy to do, I understand you got carried away. Happened
> > sometimes to me too. Still happens actually.
> 
> Found out this patch doesn't contain my removal of a duplicate section
> of code in dlgTextSearchConfiguration.cpp, so now if the name of a
> text search configuration is changed, it appears in the SQL output
> twice.  My original patch moved a whole section of code, but now the
> removal of the original copy is missing.  Everything else seems to
> work fine though.
> 

Will look into that tomorrow. Right now, I need some sleep :)


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [FEATURE] Add schema option to all relevant objects

[Thom Brown]

On 9 July 2011 00:12, Guillaume Lelarge  wrote:
> On Fri, 2011-07-08 at 21:55 +0200, Guillaume Lelarge wrote:
>> On Fri, 2011-07-08 at 20:06 +0100, Thom Brown wrote:
>> > On 8 July 2011 19:46, Guillaume Lelarge  wrote:
>> > > On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
>> > >> On 7 July 2011 23:20, Guillaume Lelarge  wrote:
>> > >> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
>> > [...]
>> > > And that should be all. The fixed patch is attached. There is one
>> > > remaining issue: how to refresh the object's parent node in the new
>> > > schema?
>> >
>> > I played around with rebuilding the node path with the new schema name
>> > to refresh it, but I kept getting endless data type issues, and have
>> > no idea how this node stuff works, so I'm not sure what to do about
>> > that.  Have any guidance?  If not, I might have another attempt at
>> > working out how to manipulate nodes.
>> >
>>
>> I'll try to look into this, but I don't think I'll be lucky.
>>
>
> I was wrong. I have it working. Code is ugly right now, so it'll need
> some cleanup/refactoring/etc. Should be able to commit it tomorrow :)

Excellent.  I'll be interested to see what you needed to do to achieve
it.  Merci beaucoup.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers