Closed Bug 418068 Opened 15 years ago Closed 15 years ago

No way to search for "date of last comment" before/after a given date

Categories

(Bugzilla :: Query/Bug List, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: gkw, Assigned: jjclark1982)

Details

(Whiteboard: [applied to b.m.o])

Attachments

(4 files, 1 obsolete file)

Currently, there is no way to search for "date of last comment" before/after a given date in bugzilla.


E.g.: Under advanced search with boolean charts, (Unconfirmed, new, assigned, reopened Calendar bugs)

NOT "Comment" -> "changed after" -> 2007-10-25

throws up a search with bug 401034 (at the bottom, correct as of writing) ("is greater than" didn't work as well)

That bug still has comments added after 2007-10-25.
The query would prove useful when searching for old bugs without any recent comment in them; the other option which works is "Last changed date" but that also includes insignificant changes like changes in the CC list.
Severity: normal → enhancement
Similar SQL query requiring a subselect for the required search above (as noted via IRC):

mysql> select bugid from (select bugs.bug_id as bugid, max(longdescs.bug_when) as lastcomment from bugs left join longdescs on bugs.bug_id = longdescs.bug_id where product_id=2 and resolution=” group by bugs.bug_id) as thetable where lastcomment < '2007-10-27';

It would be nice to be able to search this via Bugzilla itself, through the advanced search function.
Your query below actually did the right thing, but sometimes NOT is confusing. What happened is that there was *a* comment added before that date, so that matched the NOT condition. At least, I'm pretty sure that's the intended behavior.

If you try it with 2007-10-23 and that bug still shows up, then that would be a problem, and this should be reopened.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → INVALID
The problem isn't that the query didn't do what he expected, the problem is that there is no way to compose a query that does what he expects.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
I spent an hour last night trying to help him figure out how to make that query, and gave up and ended up running SQL for him to get it.

here's what he wanted to look for:

Any bugs which were still open in the Calendar product which have not had any comments added to them since 2007-10-25.

He doesn't care if there were any other changes made to them (CC list, whiteboard, whatever) only if there were comments or not.  Because the query conditions are checked separately for every comment on the bug the way it's currently set up, looking for "not comment changed after 10/25" apparently finds you bugs that have comments made before that date, whether or not they actually have any after it.
Attached patch v1 (obsolete) — Splinter Review
A big problem here is that the SQL for searching comment change date is inconsistent with other fields. This patch will give them the same behavior, which appears to negate properly.
Assignee: query-and-buglist → jjclark1982
Status: REOPENED → ASSIGNED
Attachment #304063 - Flags: review?(justdave)
Is this bug affecting only comment change date? In other words, does this bug also affect searching for "date of last flag change" or "date of last keyword added/removed" or "date of last status whiteboard change" or "date of last bug blocks/depends change", before/after a given date?
> Is this bug affecting only comment change date? In other words, does this bug
> also affect searching for "date of last flag change" or "date of last keyword
> added/removed" or "date of last status whiteboard change" or "date of last bug
> blocks/depends change", before/after a given date?

Change searches usually refer to the activity log, but comment change searches refer to the date of comment creation, so this bug only affects comment changed before/after and work time changed before/after (since work time is stored in comments).

By putting all change date conditions in the JOIN clause rather than the WHERE cause, we get the same behavior as existing change filters, including reasonable negation behavior. However, since JOIN conditions are always ANDed, it is hard to express conditions like "work time changed before 2000 OR comment included 'Y2K'" which sound simple but actually must span multiple charts in order to refer to different events.
Attachment #304063 - Attachment is obsolete: true
Attachment #304816 - Flags: review?(mkanat)
Attachment #304063 - Flags: review?(justdave)
Comment on attachment 304816 [details] [diff] [review]
v2 - fix work_time searches too

Yes, this is fine, although formatting-wise, I usually prefer to see things formatted like:

LEFT JOIN table AS $table
       ON blah = blah
          AND X = Y
          AND Z = W

Sometimes that formatting isn't practical. In that case, I still like to see the SQL keywords all right-aligned to each other. Makes for easier reading and understanding of what's going on.
Attachment #304816 - Flags: review?(mkanat) → review+
Flags: approval+
Target Milestone: --- → Bugzilla 3.2
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.156; previous revision: 1.155
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
v2 somehow got accepted despite a misnamed argument that was causing it to crash. Here is a patch against v2 to correct that and fix the formatting issue mentioned by mkanat.
Attachment #304816 - Attachment is obsolete: true
Attachment #306184 - Flags: review?(mkanat)
Comment on attachment 306184 [details] [diff] [review]
v3 - fix crash in v2

Yes, looks fine.

It got accepted because I was under the impression that you personally rigorously test all of these patches before posting them.
Attachment #306184 - Flags: review?(mkanat) → review+
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.157; previous revision: 1.156
done
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Comment on attachment 304816 [details] [diff] [review]
v2 - fix work_time searches too

since v3 was a patch against this and not a replacement, this patch is not obsolete, you need both.
Attachment #304816 - Attachment is obsolete: false
Comment on attachment 306184 [details] [diff] [review]
v3 - fix crash in v2

Was just working on backporting this to 3.0 for bmo, and this is still broken:

>+                                 "AND $table.work_time <> 0" .
>+                                 "AND $table.bug_when $operator " .
>+                                  $dbh->quote(SqlifyDate($$v)) );

You're missing a space after the <> 0 up there, so that's going to wind up looking like "<> 0AND" when it gets stitched back together and I somehow doubt that works.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Here's my backport to the 3.0 branch for use on b.m.o.  I don't know if this is worthy of taking on the branch upstream or not, guess mkanat or LpSolit can make the call on that, but I'm posting the patch anyway in case anyone else gets use out of it. :)  I'd appreciate a look-see from jjclark just for completeness, since he wrote both the patch this is backported from and the one that caused the major code differences between 3.0 and trunk in this file.
Attachment #312011 - Flags: review?(jjclark1982)
one-character patch to fix the spacing error on trunk.  So for a complete patch for the trunk you need "v2" + "v3" + this patch.
Attachment #312012 - Flags: review?(mkanat)
Attachment #312012 - Flags: review?(mkanat) → review+
trunk:

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.159; previous revision: 1.158
done
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
(In reply to comment #16)
> Created an attachment (id=312011) [details]
> Backport to 3.0 branch
> 
> Here's my backport to the 3.0 branch for use on b.m.o.  I don't know if this is
> worthy of taking on the branch upstream or not, guess mkanat or LpSolit can
> make the call on that

Thanks Dave for looking at this.

I don't know the time frame for 3.2 coming to bmo, so my wish is for this patch to land on bmo if 3.2 is some weeks or months away. The improved functionality is much needed for bugdays.
it'll be live on bmo later tonight, that's why I backported it. :)
Whiteboard: [applied to b.m.o]
Attachment #312011 - Flags: review?(jjclark1982)
Comment on attachment 312011 [details] [diff] [review]
Backport to 3.0 branch

Now that Bugzilla 3.0 is restricted to security fixes only, we are not going to take it upstream. And b.m.o now uses 3.2.
You need to log in before you can comment on or make changes to this bug.