395460 - Make multi-select fields searchable

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement, P1)

Description

[Max Kanat-Alexander]

Right now multi-select fields can't really be searched in query.cgi, because they don't show up in the bugs table.

This must be done before 3.2.

Comment 1

[Ronaldo Maia]

Attachment: v1 - romaia - Make multi-select fields searchable

This was based on the cc search

Comment 2

[Max Kanat-Alexander]

Comment on attachment 287986 [details] [diff] [review]
v1 - romaia - Make multi-select fields searchable

This looks good, but I have to test it.

What happens if you select something in the boolean charts other than equals, notequals, or changed? It looks like this needs to be written to be able to handle more charts.

Comment 3

[Ronaldo Maia]

(In reply to comment #2)
> What happens if you select something in the boolean charts other than equals,
> notequals, or changed? It looks like this needs to be written to be able to
> handle more charts.
> 

The other cases will be handled in the (?!changed) function. And I just noticed it turned out exactly like the (?:equals) function, so those can be merged.

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> The other cases will be handled in the (?!changed) function. And I just noticed
> it turned out exactly like the (?:equals) function, so those can be merged.

  Okay, so do that, attach a new patch and I'll review it.

Comment 5

[Ronaldo Maia]

Attachment: v2 - romaia

Comment 6

[Jesse Clark]

Attachment: also show multiselects in search results

I have taken the v2 patch and added a GROUP_CONCAT to show multiselect fields in search results.

Sorting search results by a multiselect is allowed, but somewhat nonsensical.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 288557 [details] [diff] [review]
also show multiselects in search results

GROUP_CONCAT is not ANSI SQL.

Comment 8

[Max Kanat-Alexander]

Comment on attachment 288509 [details] [diff] [review]
v2 - romaia

For some reason, one of the boolean charts is broken here:

I have one bug with a multi-select with the values "One" and "Two" selected.

If I do: "Multi Select" "contains any of the words" "Two One" I get every bug in the system.

The other charts seem to work OK, though I didn't test every single one.

Comment 9

[Jesse Clark]

need to replace "AND $term" with "AND ($term)" in both LEFT JOINs

Comment 10

[Jesse Clark]

Actually, placing the search criteria within the LEFT JOIN will not allow compound queries on a single chart
because all the conditions will be ANDed together. You can use "contains any of the strings" to get an OR of terms but you have to create a separate chart to get an AND of terms that appear in different rows of the multiselect table.

I find that it is much more useful to set $f in the multiselect function and let a subsequent function set $term directly.

Comment 11

[Ronaldo Maia]

Attachment: v3 - romaia

following Jesse's suggestion.

This is slightly different then what I had in mind, but I think it makes more sense:

This will search for bugs which have a option selected for the chosen multi-select and that matches the criteria.

Using mkanat previous example, If I do: "Multi Select" "contains none of the words" "One", I would still see the bug that has "One" and "Two" selected, because it has an *option* that matches the criteria.

Comment 12

[bigstijn]

(In reply to comment #11)

> Using mkanat previous example, If I do: "Multi Select" "contains none of the
> words" "One", I would still see the bug that has "One" and "Two" selected,
> because it has an *option* that matches the criteria.

contains not "0ne" = true, because contains "Two" = true ?
This is weird, for me.
Assume that keywords are implemented as multi select, would we like to have this behaviour ?  I don't think so.

If we would like to have this behaviour for keywords, we would write 2 boolean searches: contains "One"+ contains any of the values "Two Three ...".
And only in this case, the mkanat his example should be in the result list.

Comment 13

[Ronaldo Maia]

(In reply to comment #12)
> contains not "0ne" = true, because contains "Two" = true ?
> This is weird, for me.

The bug appears because it has a value that matches the criteria ("Two" does not contain the word "One"). It makes sense to me.

> Assume that keywords are implemented as multi select, would we like to have
> this behaviour ?  I don't think so.

True. Keywords are a multi-select field, but they also have a cache in the bugs table, and the searches make use of that cache.

> If we would like to have this behaviour for keywords, we would write 2 boolean
> searches: contains "One"+ contains any of the values "Two Three ...".
> And only in this case, the mkanat his example should be in the result list.

Note that "contains any of the values" is not the same as "contains any of the words" or "contains any of the strings"

Comment 14

[Max Kanat-Alexander]

Unforunately, I agree with bigstijn. If you request "contains none of the" then results that match *any* of the criteria should be excluded.

Comment 15

[Ronaldo Maia]

Attachment: v4 - romaia

Here:

is equal to => contains the value
is not equal to => does not contain the value
is equal to any of the strings => contains any of the values

Comment 16

[Max Kanat-Alexander]

Comment on attachment 295650 [details] [diff] [review]
v4 - romaia

Jesse, could I get your input on this?

Comment 17

[Jesse Clark]

Attachment: v2 - jjclark

I have updated Ronaldo's patch for bug 399371 and verified that the behavior makes sense for negative search types, compound search types, other search types, and change log searches (although changed_to can only apply to one item).

It does not break when there are no multiselect fields defined, or when many charts are used in the same search (although I had to add some parentheses).

Searching for an "and" of different values in the same chart will always return no result, but the "contains all" search type works properly .

Comment 18

[Max Kanat-Alexander]

Comment on attachment 299716 [details] [diff] [review]
v2 - jjclark

Looks good!

Comment 19

[Max Kanat-Alexander]

I gave both Romaia and Jesse credit for this patch, when I checked it in.

Comment 20

[Max Kanat-Alexander]

Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.179; previous revision: 1.178
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.153; previous revision: 1.152
done