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

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Database
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: David Taylor, Assigned: Frédéric Buclin)

Tracking

Bugzilla 4.2
Bug Flags:
approval +
blocking4.4 +
approval4.2 +
blocking4.2.3 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Keywords: qawanted
(Reporter)

Comment 1

5 years ago
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
(Assignee)

Comment 2

5 years ago
I try to remember how to list sequences and triggers in Oracle...
(Assignee)

Comment 3

5 years ago
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;
(Assignee)

Comment 4

5 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking4.4+
Flags: blocking4.2.3+
Keywords: qawanted
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Updated

5 years ago
Summary: [Oracle] DB/Schema/Oracle.pm get_add_column_ddl does not handle SERIAL → [Oracle] Adding or removing a DB column does not handle SERIAL correctly
(Assignee)

Comment 5

5 years ago
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.
Assignee: database → LpSolit
Status: NEW → ASSIGNED
Attachment #653233 - Flags: review?(dkl)
(Assignee)

Comment 6

5 years ago
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
(Assignee)

Comment 8

5 years ago
Created attachment 656627 [details] [diff] [review]
patch, v2

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+
(Assignee)

Updated

5 years ago
Flags: approval4.2+
Flags: approval+
(Assignee)

Comment 10

5 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/DB/Oracle.pm
modified Bugzilla/DB/Schema/Oracle.pm
Committed revision 8367.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/DB/Oracle.pm
modified Bugzilla/DB/Schema/Oracle.pm
Committed revision 8130.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.