bugs_activity join to fielddefs should be inner join

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Creating/Changing Bugs
--
minor
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: bbaetz, Assigned: Frédéric Buclin)

Tracking

3.0.3
Bugzilla 4.4
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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)
(Reporter)

Comment 1

10 years ago
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.
(Reporter)

Comment 2

10 years ago
Created attachment 312257 [details] [diff] [review]
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?
Assignee: database → bbaetz
Status: NEW → ASSIGNED
Attachment #312257 - Flags: review?(mkanat)
(Reporter)

Comment 3

10 years ago
Created attachment 312258 [details] [diff] [review]
With the schema change
Attachment #312257 - Attachment is obsolete: true
Attachment #312258 - Flags: review?(mkanat)
Attachment #312257 - Flags: review?(mkanat)
(Assignee)

Comment 4

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

Missing cascading on deletion, and this is a dupe anyway.
Attachment #312258 - Flags: review?(mkanat) → review-
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 414779
(Reporter)

Comment 6

10 years ago
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)
Status: RESOLVED → REOPENED
Component: Database → Creating/Changing Bugs
Depends on: 414779
Resolution: DUPLICATE → ---
Summary: fielddefs table needs FKs to it from bugs_activity → bugs_activity join to fielddefs should be inner join

Updated

10 years ago
Severity: normal → minor
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 3.2
(Assignee)

Updated

9 years ago
Status: REOPENED → NEW
(Assignee)

Comment 7

8 years ago
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).
Target Milestone: Bugzilla 3.2 → ---
(Assignee)

Comment 8

6 years ago
Created attachment 608377 [details] [diff] [review]
patch, v3
Assignee: bbaetz → LpSolit
Attachment #312258 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #608377 - Flags: review?(dkl)
(Assignee)

Updated

6 years ago
Blocks: 425618
Target Milestone: --- → Bugzilla 4.4
(Assignee)

Updated

6 years ago
Attachment #608377 - Flags: review?(dkl) → review?(timello)
Comment on attachment 608377 [details] [diff] [review]
patch, v3

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

Looks good and works as expected. r=dkl
Attachment #608377 - Flags: review+
(Assignee)

Updated

6 years ago
Attachment #608377 - Flags: review?(timello)
(Assignee)

Updated

6 years ago
Flags: approval+
(Assignee)

Comment 10

6 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Bug.pm
Committed revision 8166.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.