Closed
Bug 785565
Opened 12 years ago
Closed 11 years ago
Search by change history between two dates doesn't give expected result
Categories
(Bugzilla :: Query/Bug List, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 4.4
People
(Reporter: benoit, Assigned: LpSolit)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
When searching for bug by "Search By Change History" between two dates Bugzilla 4.2.1 and 4.2.2 (at least) list bugs that doesn't match the request.
Example when searching for bug where Status has been RESOLVED between 2012/01/01 and 2012/01/31 i got bugs that where resolved years before, or that don't even have a change recorded on this timeframe.
This doesn't happens on the 4.0.7+ of bugzilla.mozilla.org.
Extract of the sql request generated by 4.2.2:
..
LEFT JOIN bugs_activity AS act_8_1 ON bugs.bug_id = act_8_1.bug_id AND act_8_1.fieldid = 8 AND act_8_1.added = 'RESOLVED'
LEFT JOIN bugs_activity AS act_8_2 ON bugs.bug_id = act_8_2.bug_id AND act_8_2.fieldid = 8 AND act_8_2.bug_when >= '2012-01-01 00:00:00'
LEFT JOIN bugs_activity AS act_8_3 ON bugs.bug_id = act_8_3.bug_id AND act_8_3.fieldid = 8 AND act_8_3.bug_when <= '2012-01-31 23:59:59'
WHERE bugs.creation_ts IS NOT NULL
...
AND bugs.delta_ts >= '2012-01-01'
AND act_8_1.bug_when IS NOT NULL
AND act_8_2.bug_when IS NOT NULL
AND act_8_3.bug_when IS NOT NULL
..
We see three different JOINs with each a part of the filter, if i'm not misleading that request say "give me bugs whose Status ( has been changed to RESOLVED or has changed after the 2012/01/01 or has changed before the 2012/01/31 )" which is more or less all resolved bugs (modulo the filtering made by bugs.delta_ts).
Request generated on BMO:
LEFT JOIN bugs_activity AS actcheck ON (actcheck.bug_id = bugs.bug_id AND actcheck.bug_when >= '2012-01-01 00:00:00' AND actcheck.bug_when <= '2012-01-02 00:00:00' AND actcheck.added = 'RESOLVED' AND actcheck.fieldid IN (29) )
...
WHERE ((actcheck.bug_when IS NOT NULL)) ...
Which does look more like it
Reporter | ||
Comment 1•12 years ago
|
||
An attempt to fix this by reducing the joins to one per field activity.
Done by reordering the filters rules and changing the custom operators to complete the last join "extra" filters instead of creating a new join for each call.
Reporter | ||
Comment 2•12 years ago
|
||
(In reply to Benoit Plessis from comment #1)
> Created attachment 655247 [details] [diff] [review]
> proposed fix for bugzilla-4.2.2
however i didn't found how to reduce the number of ".bug_when IS NOT NULL" in the were so maybe there is a better way
Assignee | ||
Comment 3•12 years ago
|
||
If the fix is simple and doesn't regress anything, it can go into 4.2, else I will retarget it to 4.4. glob, could you look at this patch as you are already working on bug 780820?
Blocks: 780820
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Bugzilla 4.2
Assignee | ||
Updated•12 years ago
|
Attachment #655247 -
Flags: review?(glob)
Comment on attachment 655247 [details] [diff] [review]
proposed fix for bugzilla-4.2.2
this sort of hidden behavour existed in 4.0 and was removed on purpose.
bug 780820 is the right way to address this.
Attachment #655247 -
Flags: review?(glob) → review-
actually bug 677757 is the bug i was thinking of.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Updated•12 years ago
|
Target Milestone: Bugzilla 4.2 → ---
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Byron Jones ‹:glob› from comment #5)
> actually bug 677757 is the bug i was thinking of.
This is not a duplicate of 677757, though fixing it may help fixing this one. The expected behavior of the "Search By Change History" section is unambiguous and it should use one single table join matching all the conditions set in this section.
Assignee | ||
Updated•12 years ago
|
Comment 10•12 years ago
|
||
I to ran across this. This was my use case:
"Show me all of the bugs whose Status was set to Resolved between 4 weeks ago and 3 weeks ago".
What it actually delivered was all of the bugs that were ever set to Resolved, and who's delta_ts was between 4 weeks ago and 3 weeks ago.
delta_ts cannot be part of the date range query because if a bug gets changed later it won't meet the search criteria effectively. It really needs to go on bug_when in the activity log.
Also, the field change AND the date start AND the date end need to be combined into a single statement in the query, otherwise it matches them separately and the results are not what is expected.
This worked throughout 3.4, it seems to be broken this way throughout the 4.2.x release.
I came up with a total-hack fix that corrected the problem for me. I do not know if it causes other problems. All of the fixes go into Search.pm.
In _special_parse_chfield, comment out the two statements that add delta_ts to the clause. It's these two statements:
# if ($date_from ne '') {
# $clause->add('delta_ts', 'greaterthaneq', $date_from);
# }
-and-
# if ($date_to ne '' and !@fields and $value_to eq '') {
# $clause->add('delta_ts', 'lessthaneq', $date_to);
# }
Then, in _changedbefore_changedafter, there's a line that says
my $table = "act_${field_id}_$chart_id";
Change it to
my $table = "act_${field_id}_XXX";
(I don't know if I could have left the trailing XXX off, I was concerned I'd have duplicate join tables if I did this)
In _changedfrom_changedto, I did the exact same thing: find the following line:
my $table = "act_${field_id}_$chart_id";
Change it to
my $table = "act_${field_id}_XXX";
With these changes, the SQL looks like what I'd expect for a date range query on a field change and the results are consistent.
I'm not a contributor and don't create patches so I'm just submitting what I did in hopes it'll lead to new insights.
Comment 11•12 years ago
|
||
(In reply to Joe Reifel from comment #10)
> I to ran across this. This was my use case:
>
> "Show me all of the bugs whose Status was set to Resolved between 4 weeks
> ago and 3 weeks ago".
>
> What it actually delivered was all of the bugs that were ever set to
> Resolved, and who's delta_ts was between 4 weeks ago and 3 weeks ago.
We're also seeing this behavior, and with the same use case as Joe gives to reproduce, since upgrading to 4.2 (vanilla install). It's definitely not a duplicate of bug 677757, although I don't have access to make changes to the install in order to test Benoit's proposed patch or Joe's proposed fix.
Comment 12•12 years ago
|
||
My change did cause one other problem - it broke the searches where date ranges were specified but no field was specified. For example, to look for all changes in the system in the last day.
Re-pasting what I did to fix both problems.
In _special_parse_chfield, make sure it is checking for "and !@fields" in the first statement, as follows:
if ($date_from ne '' and !@fields ) {
$clause->add('delta_ts', 'greaterthaneq', $date_from);
}
Then, in _changedbefore_changedafter, there's a line that says
my $table = "act_${field_id}_$chart_id";
Change it to
my $table = "act_${field_id}_XXX";
(I don't know if I could have left the trailing XXX off, I was concerned I'd have duplicate join tables if I did this)
In _changedfrom_changedto, I did the exact same thing: find the following line:
my $table = "act_${field_id}_$chart_id";
Change it to
my $table = "act_${field_id}_XXX";
Assignee | ||
Comment 13•12 years ago
|
||
Too late for 4.4. Let's see if we can have this bug fixed for 4.4.1.
Flags: blocking4.4.1+
Flags: blocking4.4-
Flags: blocking4.4+
Target Milestone: --- → Bugzilla 4.4
Assignee | ||
Comment 14•11 years ago
|
||
We need to use Bugzilla::Search::ClauseGroup to group conditions together. If several fields are selected, we loop over them using a OR condition (because the UI says "ANY of the fields").
I checked results by requesting the DB directly and I found no problem.
Assignee: query-and-buglist → LpSolit
Attachment #655247 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #799686 -
Flags: review?(glob)
Attachment #799686 -
Flags: review?(dkl)
Comment 15•11 years ago
|
||
Comment on attachment 799686 [details] [diff] [review]
patch, v1
Review of attachment 799686 [details] [diff] [review]:
-----------------------------------------------------------------
r=glob
Attachment #799686 -
Flags: review?(glob) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #799686 -
Flags: review?(dkl)
Assignee | ||
Updated•11 years ago
|
Flags: approval?
Flags: approval4.4?
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
Assignee | ||
Comment 16•11 years ago
|
||
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 8736.
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/Search.pm
Committed revision 8606.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•