825805 - add an index to fielddefs on (custom,type,sortkey);

Status: RESOLVED WONTFIX

(Bugzilla :: Database, enhancement)

Description

[:glob ✱]

in bug 824998 sheeri has determined that the fielddefs table would benefit from an index on (custom,type,sortkey).

Comment 1

[David Lawrence [:dkl]]

Attachment: Patch to update unique index on fielddefs table (v1)

Comment 2

[Frédéric Buclin]

Comment on attachment 697934 [details] [diff] [review]
Patch to update unique index on fielddefs table (v1)

>+            fielddefs_name_idx => {FIELDS => [qw(name type sortkey)],

We want (*custom* type sortkey), not name.

Comment 3

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #2)
> Comment on attachment 697934 [details] [diff] [review]
> Patch to update unique index on fielddefs table (v1)
> 
> >+            fielddefs_name_idx => {FIELDS => [qw(name type sortkey)],
> 
> We want (*custom* type sortkey), not name.

Ugh. Yeah completely missed that. New patch coming.

Comment 4

[David Lawrence [:dkl]]

Attachment: Patch to add unique index to fielddefs table (v2)

Comment 5

[Frédéric Buclin]

Comment on attachment 698015 [details] [diff] [review]
Patch to add unique index to fielddefs table (v2)

>+            fielddefs_custom_idx  => {FIELDS => [qw(custom type sortkey)],
>+                                      TYPE => 'UNIQUE'},

Not UNIQUE! You can have two custom fields of the same type (e.g. Free Text) with the same sortkey. I'm surprised checksetup.pl didn't crash in your testing. :)

Comment 6

[David Lawrence [:dkl]]

Attachment: Patch to add new index to fielddefs table (v3)

Comment 7

[Frédéric Buclin]

I scanned our codebase, and I cannot find any place where queries of the form

 WHERE custom = 1 AND type = X

as described in bug 824998 are triggered. I added debug code into Bugzilla::Object::_do_list_select() to intercept queries generated by it, but none of these queries are of the form above. There are tons of queries of the form:

 WHERE visibility_field_id = X

and

 WHERE value_field_id = X

though. Those are generated by [% legal_value.controlled_values %] and [% field.controls_visibility_of %] in bug/field-events.js.tmpl.

I don't want to add a new index if we cannot determine which code will benefit from it.

Comment 8

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #7)
> I scanned our codebase, and I cannot find any place where queries of the form
> 
>  WHERE custom = 1 AND type = X
> 
> as described in bug 824998 are triggered.


Ah, I found it! This comes from Bugzilla::Bug::AUTOLOAD in Bugzilla 4.0:

      $self->{_multi_selects} ||= [Bugzilla->get_fields(
          {custom => 1, type => FIELD_TYPE_MULTI_SELECT })];

But AUTOLOAD is gone since Bugzilla 4.2, see bug 600123. That was the single place doing such queries. So there is no benefit in adding this new index in Bugzilla 4.2 and newer. Suggesting WONTFIX.

Comment 9

[Frédéric Buclin]

Comment on attachment 698098 [details] [diff] [review]
Patch to add new index to fielddefs table (v3)

This index is now irrelevant in Bugzilla 4.2 and newer, see my previous comment.

Comment 10

[Sheeri Cabral [:sheeri]]

great detective work....glad it's fixed in the new version!