Closed Bug 731156 Opened 13 years ago Closed 12 years ago

[Oracle] Adding or removing a DB column does not handle SERIAL correctly


(Bugzilla :: Database, defect)

Not set



Bugzilla 4.2


(Reporter: davidt, Assigned: LpSolit)



(1 file, 1 obsolete file)

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 column was created without the Oracle-compatibility sequence & trigger necessary for SERIAL-type columns.

Expected results:

DB/Schema/ 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.
Keywords: qawanted
The obvious solution is to override DB/Schema/ 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/ 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 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.
Ever confirmed: true
Flags: blocking4.4+
Flags: blocking4.2.3+
Keywords: qawanted
Target Milestone: --- → Bugzilla 4.2
Summary: [Oracle] DB/Schema/ get_add_column_ddl does not handle SERIAL → [Oracle] Adding or removing a DB column does not handle SERIAL correctly
Attached patch patch, v1 (obsolete) — Splinter Review
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.
Assignee: database → LpSolit
Attachment #653233 - Flags: review?(dkl)
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/ line 401
	Bugzilla::DB::Oracle::do(undef, 'ALTER TABLE bug_see_also ADD id integer PRIMARY KEY') called at Bugzilla/ line 606
	Bugzilla::DB::bz_add_column('Bugzilla::DB::Oracle=HASH(0x54f5ea0)', 'bug_see_also', 'id', 'HASH(0x7e06290)') called at Bugzilla/Install/ line 647
	Bugzilla::Install::DB::update_table_definitions('HASH(0x17098d0)') called at ./ line 199

According to the Oracle docs, it is not possible to add a NOT NULL column without setting a default value.

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.

Attached patch patch, v2Splinter Review
Fixes the problem dkl found while upgrading with an already populated table.
Attachment #653233 - Attachment is obsolete: true
Attachment #653233 - Flags: review?(dkl)
Attachment #656627 - Flags: review?(dkl)
Comment on attachment 656627 [details] [diff] [review]
patch, v2

Review of attachment 656627 [details] [diff] [review]:

Works as expected. r=dkl
Attachment #656627 - Flags: review?(dkl) → review+
Flags: approval4.2+
Flags: approval+
Committing to: bzr+ssh://
modified Bugzilla/DB/
modified Bugzilla/DB/Schema/
Committed revision 8367.

Committing to: bzr+ssh://
modified Bugzilla/DB/
modified Bugzilla/DB/Schema/
Committed revision 8130.
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.