Last Comment Bug 562014 - Advanced searching using "keyword" "contains none" or "does not match regex" field returns incorrect results
: Advanced searching using "keyword" "contains none" or "does not match regex" ...
Status: RESOLVED FIXED
: regression
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 3.6
: All All
: -- major with 4 votes (vote)
: Bugzilla 3.6
Assigned To: Max Kanat-Alexander
: default-qa
:
Mentors:
: 563144 (view as bug list)
Depends on: 58731
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-27 06:48 PDT by Graeme Coates
Modified: 2010-07-07 17:11 PDT (History)
7 users (show)
LpSolit: approval+
LpSolit: approval3.6+
mkanat: blocking3.6.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (1.93 KB, patch)
2010-06-23 18:34 PDT, Max Kanat-Alexander
no flags Details | Diff | Splinter Review
v2 (1.61 KB, patch)
2010-06-23 18:36 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review
v2 (3.6) (1.67 KB, patch)
2010-07-05 15:12 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review
Fix search.t for now-passing tests (2.15 KB, patch)
2010-07-07 17:01 PDT, Max Kanat-Alexander
mkanat: review+
Details | Diff | Splinter Review

Description Graeme Coates 2010-04-27 06:48:03 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2 GTB6 (.NET CLR 3.5.30729)
Build Identifier: Bugzilla 3.6

Searching using the keyword field with "contains none of the words" and "does not match regular expression" do not filter the results as expected and return results that are incorrect where a bug has more than one keyword. 

For example, I search using the condition:
"Keywords" "Contains none of the words": testing

A bug has keywords: live, testing  - this bug is returned within the list for that query. 
A bug has keywords: testing, live  - this bug is returned within the list for that query. 
A bug has keyword: testing - this bug is not returned for that query.

Using regexp searching is no better either and you can even produce some logically impossible results:

eg. advanced search: 

"Keywords" "contains any/all of the words" : testing
with advanced boolean chart search at the same time:
"Keywords" "contains none of the words" : testing

- logically this will return nothing at all times, and yet will return results here!

This is easily reproducible on landfill using either 3.6 or CVS - this is a **massive** inconvenience when you use keywords to categorise your bugs.


Reproducible: Always

Steps to Reproduce:
1. Goto advanced search 
2. Search for bugs with "keyword" "none of the words" set to a given keyword where bugs exist with that keyword in combination with another.
3. Results wrongly include results with that keyword listed (change the columns to view them)
Actual Results:  
Bugs with keywords that should be excluded are included.

Expected Results:  
If you search for a bug with "none of the words" set for the keyword field, then a bug with that word in the keywords field shouldn't be included.
Comment 1 Graeme Coates 2010-04-28 05:21:03 PDT
Further investigation - the query constructed for the above example has a series of left joins, including one to the keywords table, joining on the bug_id. For bugs with >1 keyword, you have >1 row for the bug in the keywords table. Further along in the query, the following WHERE clause excludes all the entries with the specified keyword name as follows:

WHERE ((NOT (keyworddefs_.name REGEXP '(^|[^a-z0-9])testing($|[^a-z0-9])')) AND ...

Since this acts on keyworddefs_.name (joined to keywords on the keyword_id), it fails to filter out all the rows for a bug that has multiple keywords (since you'd have multiple rows for that one bug) - thus (after the subsequent "group by bug_id" at the end of the query) you haven't got rid of all the bugs you wanted to and the bugs are wrongly included. 

This looks to be a regression due to the implementation of Bug 58731, and it looks like this was predicted by comment 42, Bug 58731 (!!) - though not only does the "regexp" option not work as in the comment, but now the "none of the words" search option for keywords is broken (where it previously worked perfectly in 3.4.6).
Comment 2 Alexey Gaidukov 2010-05-03 02:01:16 PDT
In bugzilla 3.6 advanced search "Keywords" "contains none of the words" returns no bugs without any keywords. I think it's wrong. In bugzilla 3.4 search with the same criterias returns all bugs without keywords.

https://landfill.bugzilla.org/bugzilla-3.6-branch/buglist.cgi?keywords=funny&query_format=advanced&keywords_type=nowords

returns 755 bugs

https://landfill.bugzilla.org/bugzilla-3.4-branch/buglist.cgi?keywords=funny&query_format=advanced&keywords_type=nowords

returns 8638 bugs
Comment 3 Graeme Coates 2010-05-05 02:41:35 PDT
(In reply to comment #2)
> In bugzilla 3.6 advanced search "Keywords" "contains none of the words" returns
> no bugs without any keywords. I think it's wrong. 

Agreed - it's because there is no test for NULL keywords when running "none of the words" (the LEFT JOIN to both keywords and keyworddefs results in rows with null entries in keyworddefs_.name).
Comment 4 Alex D. Baxter 2010-05-09 12:35:16 PDT
Is anyone working on a fix for this?  The issue is killing my workflow as I use various keywords to manage bugs, but I could spend a lot of time trying to patch it as my knowledge of neither Perl nor the Bugzilla codebase is recent.
Comment 5 Max Kanat-Alexander 2010-05-09 22:49:50 PDT
Confirmed by number of reports.
Comment 6 Max Kanat-Alexander 2010-05-09 22:53:45 PDT
*** Bug 563144 has been marked as a duplicate of this bug. ***
Comment 7 Max Kanat-Alexander 2010-06-23 18:34:59 PDT
Created attachment 453610 [details] [diff] [review]
v1

Okay, this fixes keywords by making them use multiselect_negative for the negative search types.
Comment 8 Max Kanat-Alexander 2010-06-23 18:36:16 PDT
Created attachment 453611 [details] [diff] [review]
v2

Oh, a tiny change--fixing the arguments wasn't necessary.
Comment 9 Frédéric Buclin 2010-07-04 16:34:16 PDT
Comment on attachment 453611 [details] [diff] [review]
v2

Works fine and looks good. It now also catches bugs with no keywords, which is fine. r=LpSolit
Comment 10 Frédéric Buclin 2010-07-04 16:34:52 PDT
Note that you have a tiny bitrot to fix before committing this patch.
Comment 11 Max Kanat-Alexander 2010-07-05 15:12:44 PDT
Created attachment 456104 [details] [diff] [review]
v2 (3.6)

3.6 requires a slightly different patch, because it doesn't have do_search_function.
Comment 12 Frédéric Buclin 2010-07-05 15:44:50 PDT
Comment on attachment 456104 [details] [diff] [review]
v2 (3.6)

r=LpSolit
Comment 13 Max Kanat-Alexander 2010-07-05 16:40:01 PDT
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 7267.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/Search.pm
Committed revision 7120.
Comment 14 Max Kanat-Alexander 2010-07-07 17:01:21 PDT
Created attachment 456361 [details] [diff] [review]
Fix search.t for now-passing tests

This fixes xt/search.t to account for the fact that certain tests are now passing that were previously marked "known broken".
Comment 15 Max Kanat-Alexander 2010-07-07 17:11:12 PDT
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified xt/lib/Bugzilla/Test/Search/Constants.pm
Committed revision 7290.

Note You need to log in before you can comment on or make changes to this bug.