414779 - Add foreign key constraints for bugs_activity table

Status: RESOLVED FIXED

(Bugzilla :: Database, enhancement)

Description

[Tony Fu]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.10) Gecko/20070510 Fedora/1.5.0.10-6.fc6 Firefox/1.5.0.10
Build Identifier: 

Add foreign key information for bugs_activity table in Bugzilla/DB/Schema.pm file.

Reproducible: Didn't try

Comment 1

[Tony Fu]

I have added foreign key information for bugs_activity table in Bugzilla/DB/Schema.pm and want to submit the patch for review.

Comment 2

[Tony Fu]

Attachment: foreign key constraints information of bugs_activity table

Hi Max,

Although I have added foreign key constraints for all tables, I am not sure what is the good way to submit them for reviewing.  I know the reviewer prefer small piece of patch, so I attached a patch which shows the foreign key information of bugs_activity table only.

If there are any other way I should do, please feel free to let me know.


Thanks,
Tony

Comment 3

[Max Kanat-Alexander]

Comment on attachment 300296 [details] [diff] [review]
foreign key constraints information of bugs_activity table 

Yes, this looks generally fine, but did you test everything in Bugzilla to make sure that these fields are correctly populated? (That is, that INSERTs and UPDATEs happen in the right order.)

Comment 4

[Max Kanat-Alexander]

Comment on attachment 300296 [details] [diff] [review]
foreign key constraints information of bugs_activity table 

>+            attach_id => {TYPE => 'INT3',
>+                          REFERENCES    =>  {TABLE  =>  'attachments',
>+                                            COLUMN  =>  'attach_id'}},

  That should also be DELETE CASCADE.

Comment 5

[Tony Fu]

(In reply to comment #3)
> (From update of attachment 300296 [details] [diff] [review])
> Yes, this looks generally fine, but did you test everything in Bugzilla to make
> sure that these fields are correctly populated? (That is, that INSERTs and
> UPDATEs happen in the right order.)
> 

I have tested them by adding and editing bugs and table updating happens in the right order.

Comment 6

[Tony Fu]

(In reply to comment #4)
> (From update of attachment 300296 [details] [diff] [review])
> >+            attach_id => {TYPE => 'INT3',
> >+                          REFERENCES    =>  {TABLE  =>  'attachments',
> >+                                            COLUMN  =>  'attach_id'}},
> 
>   That should also be DELETE CASCADE.
> 
Max,

Thank you very much for your review.

It seems that no Bugzilla UI operation (or xmlrpc) can cause an attachments.attach_id to be deleted, so I didn't put "DELETE CASCADE" for attach_id at previous patch.  But I think explicitly specify referential action
for attach_id could be a better thing to do, so I added "DELETE CASCADE" for attach_id.  Please see the new patch for details.

Thanks,
Tony


Thanks,
Tony

Comment 7

[Tony Fu]

Attachment: foreign key constraints information of bugs_activity table

Comment 8

[Max Kanat-Alexander]

Comment on attachment 302535 [details] [diff] [review]
foreign key constraints information of bugs_activity table

Yeah, this looks fine, then. Some spacing will have to be fixed on checkin.

Comment 9

[Frédéric Buclin]

Requires a branch for 3.2 before we can commit it. Do we want to delay checkins on the trunk till we have a larger number of patches ready?

Comment 10

[Max Kanat-Alexander]

(In reply to comment #9)
> Requires a branch for 3.2 before we can commit it. Do we want to delay checkins
> on the trunk till we have a larger number of patches ready?

  Sure, as long as they (like this one) don't block anything else.

Comment 12

[Max Kanat-Alexander]

I checked in all of the pending FK patches at once:

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