Closed
Bug 314364
Opened 19 years ago
Closed 15 years ago
Make QuickSearch use "matches" for comment searches instead of "substring"
Categories
(Bugzilla :: Query/Bug List, enhancement, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.6
People
(Reporter: bugreport, Assigned: mkanat)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
wicked
:
review+
|
Details | Diff | Splinter Review |
On large sites (20K bugs, 200K comments, 75M longdescs table size), the quicksearches stand out as the very slowest queries. (30-60 seconds)
Currently, they repeatedly join in charts with "comment" "substring" each_word
If we change this to "content" "matches" each_word, it speeds these searches by about a factor of 2.
CAUTION - do not break databases that do not have fulltext
Assignee | ||
Updated•18 years ago
|
Priority: -- → P4
Assignee | ||
Updated•16 years ago
|
Assignee: query-and-buglist → mkanat
Priority: P4 → P1
Summary: Quicksearch should use "matches' instead of "substring" for comments → Combine fulltext simple search and quicksearch (make QuickSearch use "matches")
Target Milestone: --- → Bugzilla 3.6
Assignee | ||
Updated•16 years ago
|
Summary: Combine fulltext simple search and quicksearch (make QuickSearch use "matches") → Make QuickSearch use "matches" for comment searches instead of "substring"
Assignee | ||
Comment 1•16 years ago
|
||
Here we go. I kept in the short_desc chart, because that allows quicksearch to still match against substrings in the short_desc. We lose matching against substrings in comments, but I think the performance improvement is worth it, and matching substrings in comments is not as important for quicksearch as matching substrings in the summary.
Attachment #374956 -
Flags: review?(wicked)
Comment 3•16 years ago
|
||
Comment on attachment 374956 [details] [diff] [review]
v1
This breaks on Pg when there are more than one search term with error
DBD::Pg::st execute failed: ERROR: GROUP BY "relevance" is ambiguous
because there are more than one "AS relevance" fields in the query. This is not a problem on simple search since that will always use exactly one "content" search term.
I'm concerned with this adding a 200 result limit to search results. While this means faster monster searches one might not get a full bug listings anymore. Although, I'm not sure bug lists with few thousand bugs are usefull.. Unfortunately there's no indication this limit is applied (this is a problem with the simple search too).
Should the search with content matches be sorted in descending relevance just like simple search does by default?
Hopefully removing quicksearch_comment_cutoff param won't introduce a performance problem but we'll see.
>Index: Bugzilla/Search/Quicksearch.pm
>===================================================================
>@@ -244,10 +242,8 @@
> elsif ($firstChar eq '#') {
>+ addChart('short_desc', 'substring', $baseWord, $negate);
>+ addChart('content', 'matches', $baseWord, $negate);
Full-text search always searches short_desc in addition to comments, so should that short_text substring dropped? Won't matches and substring produce same
>@@ -370,10 +366,7 @@
>+ addChart('content', 'matches', $word, $negate);
And same here (few lines above in the else).
Attachment #374956 -
Flags: review?(wicked) → review-
Assignee | ||
Comment 4•15 years ago
|
||
Okay, this turns out to be incredibly difficult because of the way that quicksearch works--it creates multiple charts, and if I want to create only one "content" term, I have to arbitrarily decide which chart to put it in. Unfortunately I can't "OR" charts together (as far as I'm aware) or that's what I'd do here.
Probably the only solution is to modify quicksearch so that it only creates two charts: one that does an anysubstring on all the positive words, and another that does an notanysubstring (if there is such a thing) on all the negative words, and then put the content matches search in the first chart.
Assignee | ||
Comment 5•15 years ago
|
||
Comment on attachment 374956 [details] [diff] [review]
v1
Okay, my COLUMNS patch actually made this patch work as-is, now! :-)
I also search short_desc in addition to content because content searches match only whole words, while short_desc searches can match substrings, which is the traditional (and very useful) behavior of quicksearch.
Attachment #374956 -
Flags: review- → review?(wicked)
Comment 6•15 years ago
|
||
Comment on attachment 374956 [details] [diff] [review]
v1
I would have suggested you also fix the QS docs about this change but for some reason they only mention searching the summary and nothing about doing a comment (and now content) search. So I guess there's no reason to change them for this.. Hopefully we do have that "fix QS docs" bug filed for this and other general fixup that's much needed for those two docs page.. :)
Attachment #374956 -
Flags: review?(wicked) → review+
Assignee | ||
Updated•15 years ago
|
Flags: approval+
Comment 7•15 years ago
|
||
I'm sorry I noticed so lately -- I watch review+ mails mostly.
(In reply to comment #1)
> Here we go. I kept in the short_desc chart, because that allows quicksearch to
> still match against substrings in the short_desc. We lose matching against
> substrings in comments, but I think the performance improvement is worth it,
> and matching substrings in comments is not as important for quicksearch as
> matching substrings in the summary.
I very strongly disagree. Stemming is a very strong use case. You search for "fail" so that you match "failure" as well as "failing". Bugs tend to have similar subjects, so they need to be easily distinguishable by comments.
If b.m.o needs the speedup, it should be done as a local customization imho.
Assignee | ||
Comment 8•15 years ago
|
||
Hey Marc. It's not just bmo, it's any installation of any large size, of which there are many. I think it's OK to have stemming just in the summary, and do just a full-word search in the comments. Even on medium-sized installations, the "quick" search is too slow to really be usable, with comment searches.
If we want to preserve stemming searches in the comments, we can look into fulltext engines that do that, but that would be a separate issue. Right now I just want a quicksearch that works at all for more than one search term.
Comment 9•15 years ago
|
||
There are many large installations, all right. There are many installations small enough, too, and assuming some standard distribution, I'd say there are more small ones. Useful quicksearch is pretty much broken from now on for all these. And that's not only for languages where you have compound words (don't even think of trying to search for all variants of some German word now), English is hampered way too much now, too.
If you really think this should go into Bugzilla's core, make it an option -- we have both ways available at the moment, so it shouldn't be too hard to keep both.
Assignee | ||
Comment 10•15 years ago
|
||
Mmm, I don't think it breaks useful quicksearch at all. Most of the people I've talked to aren't even aware QS searches comments--I know that I personally have always used it as a rapid summary search, and I think most people do the same.
Comment 11•15 years ago
|
||
People I'm talking to are disappointed by the Specific Search form because it can't do stemming in comments. ("I know for a fact I commented at some time something along the lines of coordinating or coordinated, but my search for coord doesn't bring it up.") I'm telling them to use Quicksearch or the Advanced Search form instead, and as a bonus, I point at Quicksearch's @-syntax, too, which makes them really happy.
So what now. I'm saying don't fix anything that isn't broken, much less for the worse. You're saying it *is* broken.
I accept that there are installations big enough for a "matches" search to make sense. Please accept that there are installations where not only a substring search is possible, but a "matches" search is hurtful. I'm confident that there are more people hurt than benefitted by your proposed change.
An additional parameter is *not* a bad thing in this case because its spread/fork ratio is really good (we're not opening up large code forks, but we allow Bugzilla to spread way further accross the field of application).
Assignee | ||
Comment 12•15 years ago
|
||
If we want stemming, we should find a fast fulltext search engine that does that.
Checking in Bugzilla/Config/Query.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/Query.pm,v <-- Query.pm
new revision: 1.7; previous revision: 1.6
done
Checking in Bugzilla/Search/Quicksearch.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v <-- Quicksearch.pm
new revision: 1.25; previous revision: 1.24
done
Checking in template/en/default/admin/params/query.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/query.html.tmpl,v <-- query.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•