If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Backport search limit from bug 632717 to bmo/4.0

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
General
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dkl, Assigned: dkl)

Tracking

Production

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Backport limiting of returned bugs for searches from bug 632717 (4.2) to BMO 4.0
(Assignee)

Comment 1

5 years ago
Created attachment 644009 [details] [diff] [review]
Patch to backport max_search_results to bmo/4.0 (v1)
Attachment #644009 - Flags: review?(glob)
Comment on attachment 644009 [details] [diff] [review]
Patch to backport max_search_results to bmo/4.0 (v1)

please augment the "### bugs found" message to show that not all results are displayed if the list is truncated.

note: to deal correctly with instances where the query matches exactly max_search_results bugs, search for a limit of max_search_results+1 and display a message if the count > max_search_results

by inspection this looks good otherwise; i'll do testing against an updated patch.
Attachment #644009 - Flags: review?(glob) → review-
(Assignee)

Comment 3

5 years ago
(In reply to Byron Jones ‹:glob› from comment #2)
> Comment on attachment 644009 [details] [diff] [review]
> Patch to backport max_search_results to bmo/4.0 (v1)
> 
> please augment the "### bugs found" message to show that not all results are
> displayed if the list is truncated.

From what I can see, we don't do this currently upstream either so we should create a separate bug for this if we add it.

> note: to deal correctly with instances where the query matches exactly
> max_search_results bugs, search for a limit of max_search_results+1 and
> display a message if the count > max_search_results

I am probably missing something here, but how would I know if the query would return exactly $max_search_results without the limit without first running it without LIMIT and then getting the row count? By adding the LIMIT $max_search_results it will always return exactly $max_search_results if it is the same or greater. Is there a way in MySQL to get the number of rows that would have been returned if a LIMIT had not been used?

Sorry to be confused here.

dkl
(Assignee)

Comment 4

5 years ago
(In reply to David Lawrence [:dkl] from comment #3)
> (In reply to Byron Jones ‹:glob› from comment #2)
> > Comment on attachment 644009 [details] [diff] [review]
> > please augment the "### bugs found" message to show that not all results are
> > displayed if the list is truncated.
> 
> From what I can see, we don't do this currently upstream either so we should
> create a separate bug for this if we add it.

I should add the only time upstream that a note is added about truncated results is when the number is about default_search_limit (500) which is separate from max_search_results.

dkl
(In reply to David Lawrence [:dkl] from comment #3)
> > please augment the "### bugs found" message to show that not all results are
> > displayed if the list is truncated.
> 
> From what I can see, we don't do this currently upstream either so we should
> create a separate bug for this if we add it.

that's fine -- i don't think the upstream behavior is correct.
 
> > note: to deal correctly with instances where the query matches exactly
> > max_search_results bugs, search for a limit of max_search_results+1 and
> > display a message if the count > max_search_results
> 
> I am probably missing something here, but how would I know if the query
> would return exactly $max_search_results without the limit without first
> running it without LIMIT and then getting the row count?

select .. limit (max_search_results + 1)
if (row-count > max_search_results) {
  print "query was limited to max_search_results"
  pop @bug_list
}
(Assignee)

Comment 6

5 years ago
Created attachment 644936 [details] [diff] [review]
Patch to backport max_search_results to bmo/4.0 (v2)

Thanks for the explanation and makes sense. New patch with suggested changes. Also allow for entering 0 for the new param disable completely. This will need to be reworked slightly when we port the changes to 4.2 as we have to take into account default search limit there which is similar but different from max search results.

dkl
Attachment #644009 - Attachment is obsolete: true
Attachment #644936 - Flags: review?(glob)
Comment on attachment 644936 [details] [diff] [review]
Patch to backport max_search_results to bmo/4.0 (v2)

r=glob with following changes to be fixed on commit

>+# In the HTML interface,  by default,  we limit the returned results, 
>+# which speeds up quite a few searches where people are really only looking
>+# for the top results.
>+if ($format->{'extension'} eq 'html' && !defined $params->param('limit')) {
>+    $params->param('limit',  Bugzilla->params->{'default_search_limit'});
>+    $vars->{'default_limited'} = 1;
>+}

remove this code -- we're not using default_search_limit

>+# When bugs returned is limited to max_search_results + 1
>+# we pop the last bug off the list (the +1) and warn the user
>+# that the results have been limited.
>+my $max_results = Bugzilla->params->{max_search_results};
>+if ($max_results && scalar @bugs == $max_results + 1) {
>+    pop @bugs;
>+    $vars->{max_search_limited} = 1;
>+}

you also need to remove the last item from @bugidlist.
Attachment #644936 - Flags: review?(glob) → review+
(Assignee)

Comment 8

5 years ago
Thanks. Changes made on commit.

Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.0
modified buglist.cgi
modified report.cgi
modified Bugzilla/Search.pm
modified Bugzilla/Config/Query.pm
modified Bugzilla/WebService/Bug.pm
modified template/en/default/admin/params/query.html.tmpl
modified template/en/default/list/list.html.tmpl                
Committed revision 8253.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Blocks: 778624
You need to log in before you can comment on or make changes to this bug.