425647 - bugs_activity join to fielddefs should be inner join

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Bradley Baetz (:bbaetz)]

bugs_activity.fieldid => fielddefs.id

justdave checked on bmo that there aren't any missing entries, and I don't think custom fields relies on it, since you can't delete fields with activity. sanitycheck.cgi already checks for it.

Also change Bug.pm's left join to fielddefs to be an inner join (and on bmo do the same for the show_activity query in User.pm)

Comment 1

[Bradley Baetz (:bbaetz)]

I traced the left join and ifnull back to rev1.45 of CGI.pl, which has the comment:

revision 1.45
date: 2000/01/22 21:43:29;  author: terry%mozilla.org;  state: Exp;  lines: +5 -3
AACK!  checksetup.pl was stomping all over the new fielddefs table if
it got run more than once.  This checkin fixes that, and also changes
the DumpBugActivity() routine to give me enough information to
hopefully repair the damaged mozilla.org database...

So the left join was to allow Terry to fix up bmo from a stuffed up schema change, about 8 years ago. I think its safe to go; patch coming.

Comment 2

[Bradley Baetz (:bbaetz)]

Attachment: Patch v1

Only catch is that there doesn't appear for there to be a way for me to catch this change as it happens and add in dummy fielddefs rows - _check_references just bails out. Theres a sanitycheck though which has been there in one form or another since the fielddefs table was added (r1.19 in sanitycheck.cgi)

I'm less confident with the link back to the bugs table, because of the whole bug deletion mess - thoughts?

Comment 3

[Bradley Baetz (:bbaetz)]

Attachment: With the schema change

Comment 4

[Frédéric Buclin]

Comment on attachment 312258 [details] [diff] [review]
With the schema change

Missing cascading on deletion, and this is a dupe anyway.

Comment 6

[Bradley Baetz (:bbaetz)]

Missed that one. OK, then lets have this just bug do the Bug.pm change. Depends on the schema change to ensure that this is safe. (bmo may want these early for bug 425665)

Comment 7

[Frédéric Buclin]

Bugzilla 3.2 is restricted to security bugs only. Moreover, this bug is either assigned to nobody or got no traction for several months now. Rather than retargetting it at each new release, I'm clearing the target milestone and the bug will be retargetted to some sensible release when someone starts fixing this bug for real (Bugzilla 3.8 more likely).

Comment 8

[Frédéric Buclin]

Attachment: patch, v3

Comment 9

[David Lawrence [:dkl]]

Comment on attachment 608377 [details] [diff] [review]
patch, v3

Review of attachment 608377 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and works as expected. r=dkl

Comment 10

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Bug.pm
Committed revision 8166.