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

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

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 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 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.

(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.

(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 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.

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?

(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.

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