products.classification_id has a different definition in an upgraded database than in a brand-new database

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Installation & Upgrading
P1
normal
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Albert Ting)

Tracking

2.19.2
Bugzilla 2.20
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

915 bytes, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
The bugzilla-tip install on landfill has a classification_id with this
definition, in the products table:

classification_id smallint(6) default '1'

Whereas the Schema creates classification_id with this definition:

classification_id smallint(6) NOT NULL default '1'

This only happens when we create the raw database, not when we add
classifications to an old install, for some reason.
(Reporter)

Updated

13 years ago
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Comment 1

13 years ago
Created attachment 179970 [details] [diff] [review]
suggested patch

Yup, adding the new field to an existing database was inconsistent.
Attachment #179970 - Flags: review?(mkanat)
(Reporter)

Comment 2

13 years ago
Comment on attachment 179970 [details] [diff] [review]
suggested patch

OK, then we also need to fix it for all the people who currently have it set
incorrectly in their DB.

Just a simple bz_change_field_type toward the end of the LEGACY CHECKS should
do it.
Attachment #179970 - Flags: review?(mkanat) → review-
(Assignee)

Comment 3

13 years ago
Created attachment 180022 [details] [diff] [review]
temporary patch

Here's an updated patch.  I'm marking it "temporary" as it will always update
the field for every checksetup.pl call.  The long term fix will occur after the
DB functions can correctly tell us when a field is set to "NOT NULL". 

Max writes:
> Unfortunately the DB functions are currently broken in their ability
> to tell if a field has changed or not.  This will be fixed
> eventually (I'm writing entirely new functions), but for right now
> there's no way to avoid it.  We need to have this change in before
> we have the patch in that fixes the problem. So I am OK with living
> with a few days of this message every time we run checksetup, on the
> tip.
Attachment #180022 - Flags: review?(mkanat)
(Assignee)

Updated

13 years ago
Attachment #179970 - Attachment is obsolete: true
(Reporter)

Comment 4

13 years ago
Comment on attachment 180022 [details] [diff] [review]
temporary patch

r=mkanat by inspection.
Attachment #180022 - Flags: review?(mkanat) → review+
(Reporter)

Updated

13 years ago
Assignee: installation → altlst
Flags: approval?
(Reporter)

Updated

13 years ago
Status: NEW → ASSIGNED
I don't like running this every time. :)  Let's find something detectable we can
tie this to before 2.20 ships.  Make sure we get a bug filed on that if there
isn't one already.
Flags: approval? → approval+
(Reporter)

Comment 6

13 years ago
The repeating message will actually be solved by bug 285111, as will any and all
repeating messages of this type, because the installation checks will become
accurate.

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.390; previous revision: 1.389
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.