287314 - Move bugs.rep_platform to use an integer instead of a varchar

Status: NEW

(Bugzilla :: Database, enhancement)

Description

[Max Kanat-Alexander]

I think that this one will be the easiest to start with.

Comment 2

[Kevin Benton]

I am unable to work this bug at this time due to other priorities.  Reassigning to default assignee.

Comment 3

[Nitish Bezzala]

Attachment: updated Install::DB and DB::Schema

Will it work for an upgrade from an enum directly to a rep_platform with ids, without the in between varchar step?

Comment 4

[Max Kanat-Alexander]

Comment on attachment 380392 [details] [diff] [review]
updated Install::DB and DB::Schema

Instead of removing lines from Install::DB, you should wrap them in a bz_column_info call that checks the type of the field and only does the change if necessary at that point in time.

Also, you can just do a selectcol_arrayref with a {Cols=>[1,2]} argument instead of doing that "prepare, execute, fetch" thing, and then put it into a hash.

Also, don't insert $dbh->quoted strings right into SQL!! Use placeholders!

Comment 5

[Nitish Bezzala]

How do I check for ENUM type? Will this work?

$dbh->bz_column_info('bugs', 'rep_platform')->{TYPE} eq "ENUM"

Comment 6

[Nitish Bezzala]

Attachment: Using selectcol_arrayref and placeholders

Comment 7

[Max Kanat-Alexander]

Comment on attachment 380593 [details] [diff] [review]
Using selectcol_arrayref and placeholders

Those aren't ENUM types there. That's not where the ENUM change happens, and you never, ever have to worry about any field ever being an ENUM, in Install::DB.

Comment 8

[Nitish Bezzala]

Attachment: Removed ENUM

Comment 9

[Frédéric Buclin]

Shouldn't Search.pm be fixed too?

Comment 10

[Max Kanat-Alexander]

Comment on attachment 408598 [details] [diff] [review]
Removed ENUM

There's a huge amount of other code that needs to be fixed in addition to this. All of Bugzilla needs to search and view the value in addition to just fixing the DB.

Also, the migration code is incorrect. You changed the field to an integer (thus destroying all data in it) before you fixed it. You need to fix the values first and then convert it to an integer.

Comment 11

[Nitish Bezzala]

Attachment: updating all bug_fields and create bug

I'm making changes for bug_severity, op_sys, priority and rep_platform together because many functions like get_legal_field_values in Bugzilla::Field are common to all of them. I am also able to create a new bug with these changes. 

Please review and let me know if I'm on the right track here.

Comment 12

[Frédéric Buclin]

Comment on attachment 408816 [details] [diff] [review]
updating all bug_fields and create bug

This looks good, but this patch alone is pretty useless as nothing can be tested. The rest of the code still needs to be fixed.