User Agent: Mozilla/5.0 (Windows NT 5.1; rv:10.0.2) Gecko/20100101 Firefox/10.0.2 Build ID: 20120215223356 Steps to reproduce: Upgrade from Bugzilla 4.0 to 4.2 Actual results: The new bug_see_also.id column was created without the Oracle-compatibility sequence & trigger necessary for SERIAL-type columns. Expected results: DB/Schema/Oracle.pm should have created a BUG_SEE_ALSO_ID_SEQ sequence and BUG_SEE_ALSO_ID_TR trigger to automatically populate the BUG_SEE_ALSO.ID field when inserting new records.
The obvious solution is to override DB/Schema/Oracle.pm to check if we are creating a new column with a SERIAL type, and call _get_create_seq_ddl($table, $field_name) as done elsewhere. However, as the broken code has now been released, I'm not sure how to create a backward-compatible change to Install/DB.pm which will work for all Oracle users: - Upgrading from 4.0 -> 4.2.1 - Upgrading from 4.0 -> 4.2.0 -> 4.2.1; or - Installing 4.2.0 then upgrading to 4.2.1; or - Installing 4.2.1
I try to remember how to list sequences and triggers in Oracle...
SELECT sequence_name FROM all_sequences WHERE sequence_owner = 'BUGS' ORDER BY sequence_name; SELECT trigger_name FROM all_triggers WHERE owner = 'BUGS' ORDER BY trigger_name;
Also, deleting such a column doesn't remove its sequence and trigger. About the bug_see_also.id column, we will have to delete and recreate the column, unless it's possible to add the sequence and trigger after the column has already been created and populated.
Created attachment 653233 [details] [diff] [review] patch, v1 This patch fixes several things: - It adds missing sequences and triggers to existing primary keys which lack them. - It fixes get_add_column_ddl() and get_drop_column_ddl() to correctly add/remove these sequences and triggers. - Various cleanup, such as fixing the indentation and moving get_drop_column_ddl() at its right place.
David: could you apply my patch to your 4.2.x installation and tell me if this correctly fixes the problem for you? If you could test some of your testcases mentioned in your comment 1, this would be great! :)
I am still not able to get this work properly when the bug_see_also table has rows of data before upgrading from 4.0 to 4.2. I get the following error: Adding new column 'id' to the 'bug_see_also' table... DBD::Oracle::db do failed: ORA-01758: table must be empty to add mandatory (NOT NULL) column (DBD ERROR: error possibly near <*> indicator at char 12 in 'ALTER TABLE <*>bug_see_also ADD id integer PRIMARY KEY') [for Statement "ALTER TABLE bug_see_also ADD id integer PRIMARY KEY"] at Bugzilla/DB/Oracle.pm line 401 Bugzilla::DB::Oracle::do(undef, 'ALTER TABLE bug_see_also ADD id integer PRIMARY KEY') called at Bugzilla/DB.pm line 606 Bugzilla::DB::bz_add_column('Bugzilla::DB::Oracle=HASH(0x54f5ea0)', 'bug_see_also', 'id', 'HASH(0x7e06290)') called at Bugzilla/Install/DB.pm line 647 Bugzilla::Install::DB::update_table_definitions('HASH(0x17098d0)') called at ./checksetup.pl line 199 According to the Oracle docs, it is not possible to add a NOT NULL column without setting a default value. http://oracleinstall.wordpress.com/2011/07/01/11g-add-a-column-with-not-null-to-the-existing-table/ Or by first creating the column as NULL, updating all rows to a default value and then adding a NOT NULL constraint to the column. A three step process. What if when adding a SERIAL type column, we add a default value of 0 automatically, then we can add the sequence and trigger after and update the column with proper incrementing values? How has this ever worked before when upgrading table definitions with Oracle from one BZ version to another? Also this issue is unrelated to the patch under review as I get the error with or without the patch when updating the DB from 4.0 to 4.2. dkl
Created attachment 656627 [details] [diff] [review] patch, v2 Fixes the problem dkl found while upgrading with an already populated table.
Comment on attachment 656627 [details] [diff] [review] patch, v2 Review of attachment 656627 [details] [diff] [review]: ----------------------------------------------------------------- Works as expected. r=dkl
Committing to: bzr+ssh://email@example.com/bugzilla/trunk/ modified Bugzilla/DB/Oracle.pm modified Bugzilla/DB/Schema/Oracle.pm Committed revision 8367. Committing to: bzr+ssh://firstname.lastname@example.org/bugzilla/4.2/ modified Bugzilla/DB/Oracle.pm modified Bugzilla/DB/Schema/Oracle.pm Committed revision 8130.