Thread: Altering column collation
Hi, Attached is a patch which fixes #328. Currently if you change a column's collation, it actually drops the column. This changes it so that it issues an ALTER TYPE to adjust the collation instead. Regards -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > [...] > Attached is a patch which fixes #328. Currently if you change a > column's collation, it actually drops the column. This changes it so > that it issues an ALTER TYPE to adjust the collation instead. > Pushed on master. Thanks for yet another patch :) I didn't push it on 1.14 yet. The patch doesn't work here, and I guess it's because I didn't push your previous patch. I'm not sure on the best move here. Should we simply remove the bug (no change detected when changing collation on the column properties dialog)? or should we apply both patches (which will give us bugfix, and better handling of columns)? Any ideas? -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Tue, 2011-07-19 at 09:32 +0200, Guillaume Lelarge wrote: > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > > [...] > > Attached is a patch which fixes #328. Currently if you change a > > column's collation, it actually drops the column. This changes it so > > that it issues an ALTER TYPE to adjust the collation instead. > > > > Pushed on master. Thanks for yet another patch :) > > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > it's because I didn't push your previous patch. I'm not sure on the best > move here. Should we simply remove the bug (no change detected when > changing collation on the column properties dialog)? or should we apply > both patches (which will give us bugfix, and better handling of > columns)? Any ideas? > Dave, I would like to know what you think of this. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: >> [...] >> Attached is a patch which fixes #328. Currently if you change a >> column's collation, it actually drops the column. This changes it so >> that it issues an ALTER TYPE to adjust the collation instead. >> > > Pushed on master. Thanks for yet another patch :) > > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > it's because I didn't push your previous patch. I'm not sure on the best > move here. Should we simply remove the bug (no change detected when > changing collation on the column properties dialog)? or should we apply > both patches (which will give us bugfix, and better handling of > columns)? Any ideas? Why can't we apply the patch to 1.14? I don't recall making any previous changes to dlgColumn.cpp. I just switched to 1.14, fast-forwarded all updates, applied the patch, built it, and it behaves correctly. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, 2011-07-19 at 20:43 +0100, Thom Brown wrote: > On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > >> [...] > >> Attached is a patch which fixes #328. Currently if you change a > >> column's collation, it actually drops the column. This changes it so > >> that it issues an ALTER TYPE to adjust the collation instead. > >> > > > > Pushed on master. Thanks for yet another patch :) > > > > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > > it's because I didn't push your previous patch. I'm not sure on the best > > move here. Should we simply remove the bug (no change detected when > > changing collation on the column properties dialog)? or should we apply > > both patches (which will give us bugfix, and better handling of > > columns)? Any ideas? > > Why can't we apply the patch to 1.14? I don't recall making any > previous changes to dlgColumn.cpp. I just switched to 1.14, > fast-forwarded all updates, applied the patch, built it, and it > behaves correctly. > We could apply it, but it doesn't work. I guess you did changes on "Prevent duplicate members' names in a composite type" that are needed for "Fix weird behaviour when changing column's collation" to make it work. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Tue, Jul 19, 2011 at 8:27 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > On Tue, 2011-07-19 at 09:32 +0200, Guillaume Lelarge wrote: >> On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: >> > [...] >> > Attached is a patch which fixes #328. Currently if you change a >> > column's collation, it actually drops the column. This changes it so >> > that it issues an ALTER TYPE to adjust the collation instead. >> > >> >> Pushed on master. Thanks for yet another patch :) >> >> I didn't push it on 1.14 yet. The patch doesn't work here, and I guess >> it's because I didn't push your previous patch. I'm not sure on the best >> move here. Should we simply remove the bug (no change detected when >> changing collation on the column properties dialog)? or should we apply >> both patches (which will give us bugfix, and better handling of >> columns)? Any ideas? >> > > Dave, I would like to know what you think of this. Err, I never even looked at the collation feature. Let's see what Thom comes up with following your comment that it applies but doesn't work, and argue about it then if necessary. Looking back at the thread though, it does seem like a bug that should be resolved. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, 2011-07-19 at 21:07 +0100, Dave Page wrote: > On Tue, Jul 19, 2011 at 8:27 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: > > On Tue, 2011-07-19 at 09:32 +0200, Guillaume Lelarge wrote: > >> On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > >> > [...] > >> > Attached is a patch which fixes #328. Currently if you change a > >> > column's collation, it actually drops the column. This changes it so > >> > that it issues an ALTER TYPE to adjust the collation instead. > >> > > >> > >> Pushed on master. Thanks for yet another patch :) > >> > >> I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > >> it's because I didn't push your previous patch. I'm not sure on the best > >> move here. Should we simply remove the bug (no change detected when > >> changing collation on the column properties dialog)? or should we apply > >> both patches (which will give us bugfix, and better handling of > >> columns)? Any ideas? > >> > > > > Dave, I would like to know what you think of this. > > Err, I never even looked at the collation feature. Let's see what Thom > comes up with following your comment that it applies but doesn't work, > and argue about it then if necessary. Looking back at the thread > though, it does seem like a bug that should be resolved. > OK. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On 19 July 2011 21:04, Guillaume Lelarge <guillaume@lelarge.info> wrote: > On Tue, 2011-07-19 at 20:43 +0100, Thom Brown wrote: >> On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: >> >> [...] >> >> Attached is a patch which fixes #328. Currently if you change a >> >> column's collation, it actually drops the column. This changes it so >> >> that it issues an ALTER TYPE to adjust the collation instead. >> >> >> > >> > Pushed on master. Thanks for yet another patch :) >> > >> > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess >> > it's because I didn't push your previous patch. I'm not sure on the best >> > move here. Should we simply remove the bug (no change detected when >> > changing collation on the column properties dialog)? or should we apply >> > both patches (which will give us bugfix, and better handling of >> > columns)? Any ideas? >> >> Why can't we apply the patch to 1.14? I don't recall making any >> previous changes to dlgColumn.cpp. I just switched to 1.14, >> fast-forwarded all updates, applied the patch, built it, and it >> behaves correctly. >> > > We could apply it, but it doesn't work. I guess you did changes on > "Prevent duplicate members' names in a composite type" that are needed > for "Fix weird behaviour when changing column's collation" to make it > work. It seems to work if you go to the table properties and edit the columns from there, but not if you go into the column directly. I tried making a change to detect the collation drop-down change, but for some reason it either still doesn't pick it up at all, or doesn't show the initial "-- nothing to change" in the SQL tab, but in that latter case it does show the correct SQL when changing collation. I'm not sure what's going on there. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, 2011-07-20 at 10:56 +0100, Thom Brown wrote: > On 19 July 2011 21:04, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > On Tue, 2011-07-19 at 20:43 +0100, Thom Brown wrote: > >> On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: > >> > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > >> >> [...] > >> >> Attached is a patch which fixes #328. Currently if you change a > >> >> column's collation, it actually drops the column. This changes it so > >> >> that it issues an ALTER TYPE to adjust the collation instead. > >> >> > >> > > >> > Pushed on master. Thanks for yet another patch :) > >> > > >> > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > >> > it's because I didn't push your previous patch. I'm not sure on the best > >> > move here. Should we simply remove the bug (no change detected when > >> > changing collation on the column properties dialog)? or should we apply > >> > both patches (which will give us bugfix, and better handling of > >> > columns)? Any ideas? > >> > >> Why can't we apply the patch to 1.14? I don't recall making any > >> previous changes to dlgColumn.cpp. I just switched to 1.14, > >> fast-forwarded all updates, applied the patch, built it, and it > >> behaves correctly. > >> > > > > We could apply it, but it doesn't work. I guess you did changes on > > "Prevent duplicate members' names in a composite type" that are needed > > for "Fix weird behaviour when changing column's collation" to make it > > work. > > It seems to work if you go to the table properties and edit the > columns from there, but not if you go into the column directly. I > tried making a change to detect the collation drop-down change, but > for some reason it either still doesn't pick it up at all, or doesn't > show the initial "-- nothing to change" in the SQL tab, but in that > latter case it does show the correct SQL when changing collation. I'm > not sure what's going on there. > Same problem here. Both on 1.14 and master. Seems like I commit a wrong patch. My bad. Trying to fix it now. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Thu, 2011-07-21 at 21:44 +0200, Guillaume Lelarge wrote: > On Wed, 2011-07-20 at 10:56 +0100, Thom Brown wrote: > > On 19 July 2011 21:04, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > > On Tue, 2011-07-19 at 20:43 +0100, Thom Brown wrote: > > >> On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > >> > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > > >> >> [...] > > >> >> Attached is a patch which fixes #328. Currently if you change a > > >> >> column's collation, it actually drops the column. This changes it so > > >> >> that it issues an ALTER TYPE to adjust the collation instead. > > >> >> > > >> > > > >> > Pushed on master. Thanks for yet another patch :) > > >> > > > >> > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > > >> > it's because I didn't push your previous patch. I'm not sure on the best > > >> > move here. Should we simply remove the bug (no change detected when > > >> > changing collation on the column properties dialog)? or should we apply > > >> > both patches (which will give us bugfix, and better handling of > > >> > columns)? Any ideas? > > >> > > >> Why can't we apply the patch to 1.14? I don't recall making any > > >> previous changes to dlgColumn.cpp. I just switched to 1.14, > > >> fast-forwarded all updates, applied the patch, built it, and it > > >> behaves correctly. > > >> > > > > > > We could apply it, but it doesn't work. I guess you did changes on > > > "Prevent duplicate members' names in a composite type" that are needed > > > for "Fix weird behaviour when changing column's collation" to make it > > > work. > > > > It seems to work if you go to the table properties and edit the > > columns from there, but not if you go into the column directly. I > > tried making a change to detect the collation drop-down change, but > > for some reason it either still doesn't pick it up at all, or doesn't > > show the initial "-- nothing to change" in the SQL tab, but in that > > latter case it does show the correct SQL when changing collation. I'm > > not sure what's going on there. > > > > Same problem here. Both on 1.14 and master. Seems like I commit a wrong > patch. My bad. Trying to fix it now. > Fixed. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Thu, 2011-07-21 at 22:54 +0200, Guillaume Lelarge wrote: > On Thu, 2011-07-21 at 21:44 +0200, Guillaume Lelarge wrote: > > On Wed, 2011-07-20 at 10:56 +0100, Thom Brown wrote: > > > On 19 July 2011 21:04, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > > > On Tue, 2011-07-19 at 20:43 +0100, Thom Brown wrote: > > > >> On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > > >> > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: > > > >> >> [...] > > > >> >> Attached is a patch which fixes #328. Currently if you change a > > > >> >> column's collation, it actually drops the column. This changes it so > > > >> >> that it issues an ALTER TYPE to adjust the collation instead. > > > >> >> > > > >> > > > > >> > Pushed on master. Thanks for yet another patch :) > > > >> > > > > >> > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess > > > >> > it's because I didn't push your previous patch. I'm not sure on the best > > > >> > move here. Should we simply remove the bug (no change detected when > > > >> > changing collation on the column properties dialog)? or should we apply > > > >> > both patches (which will give us bugfix, and better handling of > > > >> > columns)? Any ideas? > > > >> > > > >> Why can't we apply the patch to 1.14? I don't recall making any > > > >> previous changes to dlgColumn.cpp. I just switched to 1.14, > > > >> fast-forwarded all updates, applied the patch, built it, and it > > > >> behaves correctly. > > > >> > > > > > > > > We could apply it, but it doesn't work. I guess you did changes on > > > > "Prevent duplicate members' names in a composite type" that are needed > > > > for "Fix weird behaviour when changing column's collation" to make it > > > > work. > > > > > > It seems to work if you go to the table properties and edit the > > > columns from there, but not if you go into the column directly. I > > > tried making a change to detect the collation drop-down change, but > > > for some reason it either still doesn't pick it up at all, or doesn't > > > show the initial "-- nothing to change" in the SQL tab, but in that > > > latter case it does show the correct SQL when changing collation. I'm > > > not sure what's going on there. > > > > > > > Same problem here. Both on 1.14 and master. Seems like I commit a wrong > > patch. My bad. Trying to fix it now. > > > > Fixed. > And thanks, Thom, for the original patch :) -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On 21 July 2011 21:58, Guillaume Lelarge <guillaume@lelarge.info> wrote: > On Thu, 2011-07-21 at 22:54 +0200, Guillaume Lelarge wrote: >> On Thu, 2011-07-21 at 21:44 +0200, Guillaume Lelarge wrote: >> > On Wed, 2011-07-20 at 10:56 +0100, Thom Brown wrote: >> > > On 19 July 2011 21:04, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> > > > On Tue, 2011-07-19 at 20:43 +0100, Thom Brown wrote: >> > > >> On 19 July 2011 08:32, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> > > >> > On Tue, 2011-07-19 at 00:48 +0100, Thom Brown wrote: >> > > >> >> [...] >> > > >> >> Attached is a patch which fixes #328. Currently if you change a >> > > >> >> column's collation, it actually drops the column. This changes it so >> > > >> >> that it issues an ALTER TYPE to adjust the collation instead. >> > > >> >> >> > > >> > >> > > >> > Pushed on master. Thanks for yet another patch :) >> > > >> > >> > > >> > I didn't push it on 1.14 yet. The patch doesn't work here, and I guess >> > > >> > it's because I didn't push your previous patch. I'm not sure on the best >> > > >> > move here. Should we simply remove the bug (no change detected when >> > > >> > changing collation on the column properties dialog)? or should we apply >> > > >> > both patches (which will give us bugfix, and better handling of >> > > >> > columns)? Any ideas? >> > > >> >> > > >> Why can't we apply the patch to 1.14? I don't recall making any >> > > >> previous changes to dlgColumn.cpp. I just switched to 1.14, >> > > >> fast-forwarded all updates, applied the patch, built it, and it >> > > >> behaves correctly. >> > > >> >> > > > >> > > > We could apply it, but it doesn't work. I guess you did changes on >> > > > "Prevent duplicate members' names in a composite type" that are needed >> > > > for "Fix weird behaviour when changing column's collation" to make it >> > > > work. >> > > >> > > It seems to work if you go to the table properties and edit the >> > > columns from there, but not if you go into the column directly. I >> > > tried making a change to detect the collation drop-down change, but >> > > for some reason it either still doesn't pick it up at all, or doesn't >> > > show the initial "-- nothing to change" in the SQL tab, but in that >> > > latter case it does show the correct SQL when changing collation. I'm >> > > not sure what's going on there. >> > > >> > >> > Same problem here. Both on 1.14 and master. Seems like I commit a wrong >> > patch. My bad. Trying to fix it now. >> > >> >> Fixed. >> > > And thanks, Thom, for the original patch :) Cheers for fixing the problem. :) -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company