Because of Pg's wonderful ALTER TABLE limitations, we have to actually drop and re-create columns when we want to alter them or rename them. Unfortunately, this drops all indexes that were on that column, so they need to be regenerated. It should be pretty easy; we just need a function to tell us all the indexes that are on a particular column, and then we need to get the index_ddl to re-create them.

OK, here it is. I've tested this on many different columns, and it works quite well. I also fixed a bug that was causing an "uninitialized value" message to pop up during some ALTER TABLE statements on Pg.

Comment on attachment 181950 [details] [diff] [review] Re-Add indexes when altering a column in "ANSI" ALTER COLUMN > my $default = $new_def->{DEFAULT}; > my $default_old = $old_def->{DEFAULT}; >+ # This first condition prevents "uninitialized value" errors. >+ if (!defined $default && !defined $default_old) { >+ # Do Nothing >+ } > # If we went from having a default to not having one >- if (!defined $default && defined $default_old) { >+ elsif (!defined $default && defined $default_old) { > push(@statements, "ALTER TABLE $table ALTER COLUMN $column" > . " DROP DEFAULT"); > } I do not understand how this could make any difference. I believe you that this fixed the problem you had, but I would like to understand why :-). Can you explain why this works? >+=item C<get_indexes_on_column_abstract($table, $column) You forgot to close the C<> block (I have seen this on couple more places in the same module, feel free to fix all of them as part of this patch). r+, but fix the comments before checkin.

BTW, this made me think where else we need to drop/recreate indexes when touching the DB schema. What about drop_column? Shouldn't we drop/recreate the index there as well? If some databases will remove the column from multi column index and some will drop the index, we don't have consistent behaviour we need. What about rename_column?

(In reply to comment #3) > What about drop_column? [snip] Yeah, it's possible that we should also do that, but it's not as high-priority as this one is. I've only once ever run into the problem with drop_column, and it was in a pretty edge case. > What about rename_column? We don't currently have any databases that have to drop the column for a rename, so I'm not too worried about that.(In reply to comment #2) > (From update of attachment 181950 [details] [diff] [review] [edit]) > Can you explain why this works? Yeah. Look at the logic for what would have happened before if we had an undefined $default *and* an undefined $default_old. There's a "|| ($default ne $default_old)" outside of the patch context that would have run, and now never is seen. > >+=item C<get_indexes_on_column_abstract($table, $column) OK, will fix on checkin. :-)

> Yeah. Look at the logic for what would have happened before if we had an > undefined $default *and* an undefined $default_old. There's a "|| > ($default ne $default_old)" outside of the patch context that would have run, > and now never is seen. Ahhh, outside of the context... That was the trick :-). All clear now :-).

Checking in Bugzilla/DB/Schema.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v <-- Schema.pm new revision: 1.28; previous revision: 1.27 done

Here's the version that was checked-in with the checkin fixes.