On 9 July 2011 23:58, Thom Brown <t...@linux.com> wrote:
> I've found a corner-case bug btw, which requires a tiny amendment.
> But I'm also rebasing the patch for current master and 1.14_patches
> (or whatever it's called), so you'll get these shortly

Okay, here are the two rebased patches, both with the extra fix:

Patch for REL-1_14_0_PATCHES:
update_composite_type_attributes_sql_v2_rebased_1.14.patch

Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch

-- 
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 a869ccf..8b47147 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);
@@ -446,36 +447,38 @@ void dlgType::OnSelChangeTypOrLen(wxCommandEvent &ev)
 
 void dlgType::CheckChange()
 {
+	bool enable = true;
+
+	if (rdbType->GetSelection() == TYPE_COMPOSITE)
+	{
+		CheckValid(enable, lstMembers->GetItemCount() > 1, _("Please specify at least two members."));
+	}
+	else if (rdbType->GetSelection() == TYPE_ENUM)
+	{
+		CheckValid(enable, lstLabels->GetItemCount() >= 1, _("Please specify at least one label."));
+	}
+	else
+	{
+		txtLength->Enable(!chkVariable->GetValue());
+		CheckValid(enable, cbInput->GetCurrentSelection() >= 0, _("Please specify input conversion function."));
+		CheckValid(enable, cbOutput->GetCurrentSelection() >= 0, _("Please specify output conversion function."));
+		CheckValid(enable, chkVariable->GetValue() || StrToLong(txtLength->GetValue()) > 0, _("Please specify internal storage length."));
+	}
+
 	if (type)
 	{
-		EnableOK(txtComment->GetValue() != type->GetComment()
+		EnableOK(enable && (txtComment->GetValue() != type->GetComment()
 		         || cbOwner->GetValue() != type->GetOwner()
 		         || (rdbType->GetSelection() == TYPE_COMPOSITE && GetSqlForTypes() != wxEmptyString)
-		         || (GetSql().Length() > 0 && connection->BackendMinimumVersion(9, 1)));
+		         || (GetSql().Length() > 0 && connection->BackendMinimumVersion(9, 1))));
 	}
 	else
 	{
 		wxString name = GetName();
 
-		bool enable = true;
 		CheckValid(enable, !name.IsEmpty(), _("Please specify name."));
 		CheckValid(enable, !name.StartsWith(wxT("_")), _("Name may not start with '_'."));
 
-		if (rdbType->GetSelection() == TYPE_COMPOSITE)
-		{
-			CheckValid(enable, lstMembers->GetItemCount() > 1, _("Please specify at least two members."));
-		}
-		else if (rdbType->GetSelection() == TYPE_ENUM)
-		{
-			CheckValid(enable, lstLabels->GetItemCount() >= 1, _("Please specify at least one label."));
-		}
-		else
-		{
-			txtLength->Enable(!chkVariable->GetValue());
-			CheckValid(enable, cbInput->GetCurrentSelection() >= 0, _("Please specify input conversion function."));
-			CheckValid(enable, cbOutput->GetCurrentSelection() >= 0, _("Please specify output conversion function."));
-			CheckValid(enable, chkVariable->GetValue() || StrToLong(txtLength->GetValue()) > 0, _("Please specify internal storage length."));
-		}
 		EnableOK(enable);
 	}
 }
@@ -523,7 +526,7 @@ void dlgType::OnMemberAdd(wxCommandEvent &ev)
 		type += wxT(")");
 	}
 
-	if (!name.IsEmpty())
+	if (!name.IsEmpty() && lstMembers->FindItem(-1, name, false) == -1)
 	{
 		size_t pos = lstMembers->GetItemCount();
 		lstMembers->InsertItem(pos, name, 0);
@@ -533,6 +536,7 @@ void dlgType::OnMemberAdd(wxCommandEvent &ev)
 		memberLengths.Add(length);
 		memberPrecisions.Add(precision);
 		memberCollations.Add(collation);
+		memberOriginalNames.Add(wxEmptyString);
 	}
 
 	CheckChange();
@@ -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,45 +847,108 @@ 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_full_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;
+	int    hold = 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 || old_name != new_name
-		           || old_type != new_type
-		           || old_collation != new_collation;
-	}
+		// this will decide whether we progress to the next new item,
+		// or whether we need to continue checking the old list first
+		hold = 0;
 
-	if (modified)
-	{
-		// Drop all old attributes
-		for (i = 0 ; i < elements.GetCount() ; i += 3)
+		// these are a copy of the list before any changes
+		if (elements.GetCount() >= (oldindex * 3) + 3)
+		{
+			old_name = elements.Item(oldindex * 3);
+			old_type = elements.Item(oldindex * 3 + 1);
+			old_collation = elements.Item(oldindex * 3 + 2);
+		}
+		else
+		{
+			// we've now used up all the old attributes
+			old_name = wxEmptyString;
+			old_type = wxEmptyString;
+			old_collation = wxEmptyString;
+		}
+
+		// this is the original name of the type before editing
+		original_name = memberOriginalNames.Item(newindex);
+
+		new_name = lstMembers->GetItemText(newindex);
+		new_type = memberTypes.Item(newindex).AfterFirst(':');
+		new_full_type = GetFullTypeName(newindex);
+		new_collation = memberCollations.Item(newindex);
+
+		if (!original_name.IsEmpty() && original_name == old_name && (new_name != old_name
+		    || new_type != old_type || new_collation != old_collation))
+		{
+			// if this was originally in the list and the name has changed then rename it
+
+			if (new_name != old_name)
+			{
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  RENAME ATTRIBUTE ")
+				       + qtIdent(original_name) + wxT(" TO ") + qtIdent(new_name) + wxT(";\n");
+			}
+
+			if (new_type != old_type || new_collation != old_collation)
+			{
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  ALTER ATTRIBUTE ")
+				       + qtIdent(new_name);
+
+				// the syntax for alter attribute requires that we always specify the type
+				sql += wxT(" SET DATA TYPE ") + new_type;
+
+				if (new_collation != old_collation)
+					sql += wxT(" COLLATE ") + new_collation;
+
+				sql += wxT(";\n");
+			}
+		}
+		else if (!original_name.IsEmpty() && original_name != old_name)
+		{
+			// the old attribute isn't in the new list so drop it
+
+			// don't move through new list yet
+			hold = 1;
+
+			sql += wxT("ALTER TYPE ") + objname + wxT("\n  DROP ATTRIBUTE ")
+			       + qtIdent(old_name) + wxT(";\n");
+		}
+		else if (original_name.IsEmpty())
+		{
+			if (!old_name.IsEmpty())
+			{
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  DROP ATTRIBUTE ")
+				       + qtIdent(old_name) + wxT(";\n");
+			}
+
+			sql += wxT("ALTER TYPE ") + objname + wxT("\n  ADD ATTRIBUTE ")
+			       + qtIdent(new_name) + wxT(" ") + new_full_type + wxT(";\n");
+		}
+		else
 		{
-			old_name = elements.Item(i);
-			sql += wxT("ALTER TYPE ") + type->GetName() + wxT(" DROP ATTRIBUTE ") + old_name + wxT(";\n");
+			// do nothing
 		}
 
-		// Add all new attributes
-		for (int i = 0 ; i < lstMembers->GetItemCount() ; i++)
+		oldindex++;
+
+		if (newindex + 1 - hold == lstMembers->GetItemCount() && elements.GetCount() >= (oldindex * 3) + 3)
 		{
-			new_name = lstMembers->GetItemText(i);
-			new_type = GetFullTypeName(i);
-			new_collation = memberCollations.Item(i);
-			sql += wxT("ALTER TYPE ") + type->GetName() + wxT(" ADD ATTRIBUTE ")
-			       + new_name + wxT(" ") + new_type;
-			if (!new_collation.IsEmpty() && new_collation != wxT("pg_catalog.\"default\""))
-				sql += wxT(" COLLATE ") + new_collation;
-			sql += wxT(";\n");
+			// remove remaining old attributes
+			for (;elements.GetCount() >= (oldindex * 3) + 3;oldindex++)
+			{
+				old_name = elements.Item(oldindex * 3);
+				old_type = elements.Item(oldindex * 3 + 1);
+				old_collation = elements.Item(oldindex * 3 + 2);
+
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  DROP ATTRIBUTE ")
+				       + qtIdent(old_name) + wxT(";\n");
+			}
+			break;
 		}
 	}
 
diff --git a/pgadmin/include/dlg/dlgType.h b/pgadmin/include/dlg/dlgType.h
index d618221..8a3481b 100644
--- a/pgadmin/include/dlg/dlgType.h
+++ b/pgadmin/include/dlg/dlgType.h
@@ -54,7 +54,7 @@ private:
 	void showDefinition(int panel);
 	wxString GetFullTypeName(int type);
 
-	wxArrayString memberTypes, memberLengths, memberPrecisions, memberCollations;
+	wxArrayString memberTypes, memberLengths, memberPrecisions, memberCollations, memberOriginalNames;
 	bool queriesToBeSplitted;
 
 	DECLARE_EVENT_TABLE()
diff --git a/pgadmin/dlg/dlgType.cpp b/pgadmin/dlg/dlgType.cpp
index b3f35a4..3dca71a 100644
--- a/pgadmin/dlg/dlgType.cpp
+++ b/pgadmin/dlg/dlgType.cpp
@@ -286,6 +286,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);
@@ -447,38 +448,40 @@ void dlgType::OnSelChangeTypOrLen(wxCommandEvent &ev)
 
 void dlgType::CheckChange()
 {
+	bool enable = true;
+
+	if (rdbType->GetSelection() == TYPE_COMPOSITE)
+	{
+		CheckValid(enable, lstMembers->GetItemCount() > 1, _("Please specify at least two members."));
+	}
+	else if (rdbType->GetSelection() == TYPE_ENUM)
+	{
+		CheckValid(enable, lstLabels->GetItemCount() >= 1, _("Please specify at least one label."));
+	}
+	else
+	{
+		txtLength->Enable(!chkVariable->GetValue());
+		CheckValid(enable, cbInput->GetCurrentSelection() >= 0, _("Please specify input conversion function."));
+		CheckValid(enable, cbOutput->GetCurrentSelection() >= 0, _("Please specify output conversion function."));
+		CheckValid(enable, chkVariable->GetValue() || StrToLong(txtLength->GetValue()) > 0, _("Please specify internal storage length."));
+	}
+
 	if (type)
 	{
-		EnableOK(txtName->GetValue() != type->GetName()
+		EnableOK(enable && (txtName->GetValue() != type->GetName()
 				 || txtComment->GetValue() != type->GetComment()
 		         || cbSchema->GetValue() != type->GetSchema()->GetName()
 		         || cbOwner->GetValue() != type->GetOwner()
 		         || (rdbType->GetSelection() == TYPE_COMPOSITE && GetSqlForTypes() != wxEmptyString)
-		         || (GetSql().Length() > 0 && connection->BackendMinimumVersion(9, 1)));
+		         || (GetSql().Length() > 0 && connection->BackendMinimumVersion(9, 1))));
 	}
 	else
 	{
 		wxString name = GetName();
 
-		bool enable = true;
 		CheckValid(enable, !name.IsEmpty(), _("Please specify name."));
 		CheckValid(enable, !name.StartsWith(wxT("_")), _("Name may not start with '_'."));
 
-		if (rdbType->GetSelection() == TYPE_COMPOSITE)
-		{
-			CheckValid(enable, lstMembers->GetItemCount() > 1, _("Please specify at least two members."));
-		}
-		else if (rdbType->GetSelection() == TYPE_ENUM)
-		{
-			CheckValid(enable, lstLabels->GetItemCount() >= 1, _("Please specify at least one label."));
-		}
-		else
-		{
-			txtLength->Enable(!chkVariable->GetValue());
-			CheckValid(enable, cbInput->GetCurrentSelection() >= 0, _("Please specify input conversion function."));
-			CheckValid(enable, cbOutput->GetCurrentSelection() >= 0, _("Please specify output conversion function."));
-			CheckValid(enable, chkVariable->GetValue() || StrToLong(txtLength->GetValue()) > 0, _("Please specify internal storage length."));
-		}
 		EnableOK(enable);
 	}
 }
@@ -526,7 +529,7 @@ void dlgType::OnMemberAdd(wxCommandEvent &ev)
 		type += wxT(")");
 	}
 
-	if (!name.IsEmpty())
+	if (!name.IsEmpty() && lstMembers->FindItem(-1, name, false) == -1)
 	{
 		size_t pos = lstMembers->GetItemCount();
 		lstMembers->InsertItem(pos, name, 0);
@@ -536,6 +539,7 @@ void dlgType::OnMemberAdd(wxCommandEvent &ev)
 		memberLengths.Add(length);
 		memberPrecisions.Add(precision);
 		memberCollations.Add(collation);
+		memberOriginalNames.Add(wxEmptyString);
 	}
 
 	CheckChange();
@@ -599,6 +603,7 @@ void dlgType::OnMemberRemove(wxCommandEvent &ev)
 		memberLengths.RemoveAt(pos);
 		memberPrecisions.RemoveAt(pos);
 		memberCollations.RemoveAt(pos);
+		memberOriginalNames.RemoveAt(pos);
 	}
 	CheckChange();
 }
@@ -840,7 +845,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;
@@ -849,45 +854,108 @@ 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_full_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;
+	int    hold = 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 || old_name != new_name
-		           || old_type != new_type
-		           || old_collation != new_collation;
-	}
+		// this will decide whether we progress to the next new item,
+		// or whether we need to continue checking the old list first
+		hold = 0;
 
-	if (modified)
-	{
-		// Drop all old attributes
-		for (i = 0 ; i < elements.GetCount() ; i += 3)
+		// these are a copy of the list before any changes
+		if (elements.GetCount() >= (oldindex * 3) + 3)
+		{
+			old_name = elements.Item(oldindex * 3);
+			old_type = elements.Item(oldindex * 3 + 1);
+			old_collation = elements.Item(oldindex * 3 + 2);
+		}
+		else
+		{
+			// we've now used up all the old attributes
+			old_name = wxEmptyString;
+			old_type = wxEmptyString;
+			old_collation = wxEmptyString;
+		}
+
+		// this is the original name of the type before editing
+		original_name = memberOriginalNames.Item(newindex);
+
+		new_name = lstMembers->GetItemText(newindex);
+		new_type = memberTypes.Item(newindex).AfterFirst(':');
+		new_full_type = GetFullTypeName(newindex);
+		new_collation = memberCollations.Item(newindex);
+
+		if (!original_name.IsEmpty() && original_name == old_name && (new_name != old_name
+		    || new_type != old_type || new_collation != old_collation))
+		{
+			// if this was originally in the list and the name has changed then rename it
+
+			if (new_name != old_name)
+			{
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  RENAME ATTRIBUTE ")
+				       + qtIdent(original_name) + wxT(" TO ") + qtIdent(new_name) + wxT(";\n");
+			}
+
+			if (new_type != old_type || new_collation != old_collation)
+			{
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  ALTER ATTRIBUTE ")
+				       + qtIdent(new_name);
+
+				// the syntax for alter attribute requires that we always specify the type
+				sql += wxT(" SET DATA TYPE ") + new_type;
+
+				if (new_collation != old_collation)
+					sql += wxT(" COLLATE ") + new_collation;
+
+				sql += wxT(";\n");
+			}
+		}
+		else if (!original_name.IsEmpty() && original_name != old_name)
+		{
+			// the old attribute isn't in the new list so drop it
+
+			// don't move through new list yet
+			hold = 1;
+
+			sql += wxT("ALTER TYPE ") + objname + wxT("\n  DROP ATTRIBUTE ")
+			       + qtIdent(old_name) + wxT(";\n");
+		}
+		else if (original_name.IsEmpty())
+		{
+			if (!old_name.IsEmpty())
+			{
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  DROP ATTRIBUTE ")
+				       + qtIdent(old_name) + wxT(";\n");
+			}
+
+			sql += wxT("ALTER TYPE ") + objname + wxT("\n  ADD ATTRIBUTE ")
+			       + qtIdent(new_name) + wxT(" ") + new_full_type + wxT(";\n");
+		}
+		else
 		{
-			old_name = elements.Item(i);
-			sql += wxT("ALTER TYPE ") + type->GetName() + wxT("\n  DROP ATTRIBUTE ") + old_name + wxT(";\n");
+			// do nothing
 		}
 
-		// Add all new attributes
-		for (int i = 0 ; i < lstMembers->GetItemCount() ; i++)
+		oldindex++;
+
+		if (newindex + 1 - hold == lstMembers->GetItemCount() && elements.GetCount() >= (oldindex * 3) + 3)
 		{
-			new_name = lstMembers->GetItemText(i);
-			new_type = GetFullTypeName(i);
-			new_collation = memberCollations.Item(i);
-			sql += wxT("ALTER TYPE ") + type->GetName() + wxT("\n  ADD ATTRIBUTE ")
-			       + new_name + wxT(" ") + new_type;
-			if (!new_collation.IsEmpty() && new_collation != wxT("pg_catalog.\"default\""))
-				sql += wxT(" COLLATE ") + new_collation;
-			sql += wxT(";\n");
+			// remove remaining old attributes
+			for (;elements.GetCount() >= (oldindex * 3) + 3;oldindex++)
+			{
+				old_name = elements.Item(oldindex * 3);
+				old_type = elements.Item(oldindex * 3 + 1);
+				old_collation = elements.Item(oldindex * 3 + 2);
+
+				sql += wxT("ALTER TYPE ") + objname + wxT("\n  DROP ATTRIBUTE ")
+				       + qtIdent(old_name) + wxT(";\n");
+			}
+			break;
 		}
 	}
 
diff --git a/pgadmin/include/dlg/dlgType.h b/pgadmin/include/dlg/dlgType.h
index d618221..8a3481b 100644
--- a/pgadmin/include/dlg/dlgType.h
+++ b/pgadmin/include/dlg/dlgType.h
@@ -54,7 +54,7 @@ private:
 	void showDefinition(int panel);
 	wxString GetFullTypeName(int type);
 
-	wxArrayString memberTypes, memberLengths, memberPrecisions, memberCollations;
+	wxArrayString memberTypes, memberLengths, memberPrecisions, memberCollations, memberOriginalNames;
 	bool queriesToBeSplitted;
 
 	DECLARE_EVENT_TABLE()
-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Reply via email to