109528 - Can't query for attachment status != value if patch has no statuses

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect, P1)

Description

[Dave Miller [:justdave]]

If you query for patches that don't have a particular status value, the query
will fail to match patches that don't have any statuses yet.

Comment 1

[Dave Miller [:justdave]]

I tried this using "not equal to", "does not contain", and "none of the words"
and got differing results, but all failed to match patches that didn't have any
statuses yet.

Comment 2

[Dave Miller [:justdave]]

Hmm, I think querying on attachment status is generally broken still.

I just ran the following query:

http://bugzilla.mozilla.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&product=Bugzilla&field0-0-0=attachments.ispatch&type0-0-0=equals&value0-0-0=1&field0-1-0=attachments.isobsolete&type0-1-0=notequals&value0-1-0=1&field0-2-0=attachstatusdefs.name&type0-2-0=equals&value0-2-0=first-review&field0-3-0=attachstatusdefs.name&type0-3-0=notequals&value0-3-0=second-review&field0-4-0=attachstatusdefs.name&type0-4-0=notequals&value0-4-0=needs-work

which broken down so you can read it a little easier is:

http://bugzilla.mozilla.org/buglist.cgi
?bug_status=NEW
&bug_status=ASSIGNED
&bug_status=REOPENED
&product=Bugzilla
&field0-0-0=attachments.ispatch   &type0-0-0=equals   &value0-0-0=1
&field0-1-0=attachments.isobsolete&type0-1-0=notequals&value0-1-0=1
&field0-2-0=attachstatusdefs.name &type0-2-0=equals   &value0-2-0=first-review
&field0-3-0=attachstatusdefs.name &type0-3-0=notequals&value0-3-0=second-review
&field0-4-0=attachstatusdefs.name &type0-4-0=notequals&value0-4-0=needs-work

This matched the following bugs:

bug 73502: has both first-review and second-review on the same and only patch.
           (second-review was excluded in the query)
bug 82143: a correct match - only one patch, and it only has first-review
bug 83033: has 5 patches, only one is not obsolete, and that one has both
           first-review and needs-work on it (note that needs-work is excluded
           in the query)
bug 86051: has 3 patches, only one is not obsolete, and that one has both
           first-review and needs-work on it.
bug 97234: has 2 patches, only one is not obsolete, and that one has both
           first-review and needs-work on it.
bug 103568: has only one patch, and it has both first-review and needs-work on
            it.
bug 109530: a correct match - has 2 patches, one is obsolete, the other one has
            first-review on it.

Comment 3

[Gervase Markham [:gerv]]

Here's the SQL - but I'm not good at reading this stuff.

SELECT bugs.bug_id, bugs.groupset, unix_timestamp(bugs.delta_ts),
substring(bugs.bug_severity, 1, 3), substring(bugs.rep_platform, 1, 3),
map_assigned_to.login_name, substring(bugs.bug_status,1,4),
substring(bugs.resolution,1,4), substring(bugs.component, 1, 8),
bugs.target_milestone, substring(bugs.short_desc, 1, 60) 

FROM bugs, profiles map_assigned_to, profiles map_reporter 

LEFT JOIN profiles map_qa_contact 
  ON bugs.qa_contact = map_qa_contact.userid, attachments attachments_0,
     attachstatuses attachstatuses_0_1, attachstatusdefs attachstatusdefs_0_1, 
      attachstatuses attachstatuses_0_2, attachstatusdefs attachstatusdefs_0_2,
       attachstatuses attachstatuses_0_3, attachstatusdefs attachstatusdefs_0_3 

LEFT JOIN cc selectVisible_cc 
  ON bugs.bug_id = selectVisible_cc.bug_id 

AND selectVisible_cc.who = 9911 

WHERE 
  (
    (bugs.groupset & 3321) = bugs.groupset 
    OR (bugs.reporter_accessible = 1 
      AND bugs.reporter = 9911) 
    OR (bugs.assignee_accessible = 1 
      AND bugs.assigned_to = 9911)
    OR (bugs.qacontact_accessible = 1 
      AND bugs.qa_contact = 9911) 
    OR (bugs.cclist_accessible = 1 
      AND selectVisible_cc.who = 9911 
      AND not isnull(selectVisible_cc.who))
  )
   
  AND bugs.assigned_to = map_assigned_to.userid 
  AND bugs.reporter = map_reporter.userid 
  AND bugs.bug_id = attachments_0.bug_id 
  AND bugs.bug_id = attachments_0.bug_id 
  AND bugs.bug_id = attachments_0.bug_id 
  AND attachments_0.attach_id = attachstatuses_0_1.attach_id 
  AND attachstatuses_0_1.statusid = attachstatusdefs_0_1.id 
  AND bugs.bug_id = attachments_0.bug_id 
  AND attachments_0.attach_id = attachstatuses_0_2.attach_id 
  AND attachstatuses_0_2.statusid = attachstatusdefs_0_2.id 
  AND bugs.bug_id = attachments_0.bug_id 
  AND attachments_0.attach_id = attachstatuses_0_3.attach_id 
  AND attachstatuses_0_3.statusid = attachstatusdefs_0_3.id 
  
  AND (bugs.bug_status = 'NEW' 
    OR bugs.bug_status = 'ASSIGNED' 
    OR bugs.bug_status = 'REOPENED') 
  AND (bugs.product = 'Bugzilla') 
  
  AND (attachments_0.ispatch = '1') 
  AND (attachments_0.isobsolete != '1') 
  AND (attachstatusdefs_0_1.name = 'first-review') 
  AND (attachstatusdefs_0_2.name != 'second-review') 
  AND (attachstatusdefs_0_3.name != 'needs-work') 

GROUP BY bugs.bug_id 

ORDER BY bugs.short_desc, bugs.bug_id

Gerv

Comment 4

[Bradley Baetz (:bbaetz)]

I think (and I'm going to have to test) that the problem may be that we're
matching on the same attachment status. The problem is that if a patch has
first-review, second review, then is also has a status of first-review, _and_ a
status which is not second-review (the first-review one).

In other words, you're asking the wrong question. What question you should be
asking (and how to represent this in sql) is a different issue.

Does that sound logical?

Comment 5

[Bradley Baetz (:bbaetz)]

OK, there are two problems (I still haven't played directly with SQL)

First is that the joins to the attachment status tables need to be LEFT JOINs,
otherwise bugs with no attachment statuses aren't considered. This one is easy
to fix, I think.

Secondly, the problem is that we're saying: Is there an attachment status !=
'value', where what we mean is: "Is there an attachment which does not have an
entry for 'value' as an attachment status". I'll play with this tomorrow.

Comment 6

[Bradley Baetz (:bbaetz)]

Note that this 'works' for keywords because we use the keyword cache for most
things, like "Select none" (Select any/all work using the keywords table, but
that operation restricts the type of results you want, so...)

I'm going to have to think some more about this one. One question though: Do we
all agree that the various options in the boolean charts bits should apply to
each attachment status? Currently if I have the keywords k1 & k2 on a bug, then
searching for keywords contains "kw1, kw2" will find the bug, because we search
the string. Personally, I consider this a bug. Do others agree?

Oh, and should searching for bugs w/o attachment status of 'foo' find bugs with
no attachments?

IIRC, this sort of complication was one of the main reasons for having a
keywords cache. Since I don't think that we should have an attachment status
cache, we're going to have to find a solution to this. (Then we can maybe remove
the keyword cache... ;)

Comment 7

[Bradley Baetz (:bbaetz)]

Taking. I have a patch, which fixes the problem, sort of. The main problem is
that this sort of thing is undefined:

# eg, does 'contains all' mean that the status has to contain all
# those words, or that all those words must be exact matches to
# statuses, which must all be on a single attachment, or should
# the match on the status descriptions be a contains match, too?

Personally, I think that we should change the meaning of contains so that
for keywords, attachent statuses, and the new groups stuff, each (possibly
quoted) word matches one of the statuses, while not equals would mean that the
attachment status was not _exclusively_ the given quote. Thats a bigger change,
and definately post 2.16.

Anyway, the patch is really ugly, but I don't think that there is a better
solution in this timespan

Comment 8

[Bradley Baetz (:bbaetz)]

Attachment: patch

Comment 9

[Matthew Tuck [:CodeMachine]]

Said issue is why I filed bug #75396 as the long-term plan.

Comment 10

[Gervase Markham [:gerv]]

(Just a note: I can't review this because I don't understand this code at all.)

Gerv

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 67567 [details] [diff] [review]
patch

This patch fails to apply on the tip.  bbaetz- can you post an update?

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: applies to the tip

This version of the patch applies to the tip.  I didn't change anything about
bbaetz's patch, I just got it to apply.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 76785 [details] [diff] [review]
patch v2: applies to the tip

Works.	r=myk

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Hmm, we've been living with this for a while.  We can continue to live with it.
 Nice-to-have, not a blocker.

Comment 15

[Dave Miller [:justdave]]

Comment on attachment 76785 [details] [diff] [review]
patch v2: applies to the tip

r= justdave

it works! :)

Mozilla doesn't though (I'll file that elsewhere, for a peek at my fun, see
http://www.justdave.net/Picture1.jpg )

Comment 16

[Bradley Baetz (:bbaetz)]

Checked in.