743883 - In boolean charts, "<user field> is not equal to [e-mail]" only returns bugs which totally lack this user

Status: NEW

Description

[Frédéric Buclin]

The rewrite of Search.pm in 4.2 seems to have happily messed the definition of
  "<user field> is not equal to [e-mail]"

It now seems to behave exactly the same way as
  "NOT <user field> is equal to [e-mail]"

which is only correct for single-value user fields (reporter, assignee, QA contact?), but not for multi-values user fields (commenters, CC members, attachment creators, flag setters and requestees). The bad news is that xt tests have been written with this bad logic in mind.


There are 4 cases to consider:

1. Field F is equal to value V
==============================

This means that the field's value must be V if it's a single-value field, or must contain V if it's a multi-values field. I think we all agree on both parts of this definition.


2. Field F is not equal to value V
==================================

For single-value fields, this means that the field's value must not be V. We all agree about this. For multi-values fields, this means that V must not be one of the values. We also all agree about this. But for user fields such as commenters, CC members, attachment creators, etc..., we don't agree. When looking for bugs where [e-mail] is not an attachment creator or not a commenter, there are two ways to understand this:

2.1. [e-mail] is not one of the attachment creators/commenters
2.2. [e-mail] is not the single attachment creator/commenter

From a mathematical logic point of view, the correct query for 2.1 is:
  "NOT(<user field> is equal to [e-mail])"

and the correct query for 2.2 is:
  "<user field> is not equal to [e-mail]"

And even with this distinction, there is some disagreement about bugs with no attachments (or flags) at all (all bugs have at least one comment, a.k.a. description or comment 0, so that's not a problem here). Should a bug with no attachments or flags be returned in query 2.1? And in query 2.2? IMO, 2.1 should return bugs with no attachments/flags, because the definition of NOT() is to return all bugs minus bugs matching the query in parens, but 2.2 shouldn't, because we are talking about an attribute of an object, and so this only makes sense if this object exists. So they are not equivalent.

"How many men have a wife with no blue eyes?" should only include men who have a wife, but with no blue eyes. This question shouldn't include men with no wife, because it doesn't make sense to talk about the color of eyes of someone who doesn't exist. The same is true for attachments and flags. It doesn't make sense to talk about the creator of an attachment or the requester of a flag if this attachment or flag doesn't exist.


3. NOT(field F is equal to value V)
===================================

This has been discussed in 2.1 above. It should return all bugs which do not match the query in parens. This includes bugs which do not have this attribute at all (see the discussion about bugs with no attachments or flags at all). No idea if we all agree on this definition or not, but the mathematical definition of NOT() should win.


4. NOT(field F is not equal to value V)
=======================================

The double negation means that only bugs where field F is exactly V should be returned. But for bugs where this field doesn't exist (bugs with no attachments or flags), should they also be returned or not? Again some disagreement here. As this is the exact opposite story of 2.1 and 2.2, it all depends on how we consider things there.


So for now, I consider xt tests with a grain of salt (some are cleary wrong). I think that following very carefully the definitions above would save us a lot of trouble. And arguing that people wouldn't expect some of these behaviors won't make the search code any better. They should rather revise their maths, or avoid building too complex queries with boolean charts. If this helps, the documentation should be improved with very clear examples, as I did for the QuickSearch documentation, see https://landfill.bugzilla.org/bugzilla-tip/page.cgi?id=quicksearch.html#advanced_examples

Comment 1

[Max Kanat-Alexander]

The reason for this behavior is that it is the normal intended behavior from my experience with Bugzilla users--in particular, think of all the times users wanted to search "cc is not equal to" and we had to explain to them the very-confusing NOT(cc is equal to) syntax.

However, I do understand your use case. The solution that I planned to do was to add different fields for "All X" and "Any X". (That may not be the final UI terminology, it's just a way to think about it.)

For example, "All Comments do not contain 'foo'" would find you bugs where no comment contained "foo". But "Any comment does not contain foo" would find you bugs where at least one comment did not contain foo.

Ultimately we want to support both of these use cases. What I did was modify Search.pm to actually behave consistently instead of magically and confusingly, and now it's time to update and clarify the UI so that it has the capabilities that users want for other things.

Comment 2

[Frédéric Buclin]

(In reply to Max Kanat-Alexander from comment #1)
> and now it's time to update and clarify the UI so that it has
> the capabilities that users want for other things.

Yes I agree. :) Because having a version-dependent behavior is a bit painful (some queries work differently in 3.6, 4.0 and 4.2, when using negation operators).

Comment 3

[Max Kanat-Alexander]

Yeah. Now with 4.2 we have a good base where all the operators for the current fields behave the same way, at least. From here on out it's just a matter of adding new functionality for the other things that people want, in the way that we can demonstrate they actually need. (And I do agree that people need the "All X/Any X" stuff I was talking about.)

Comment 4

[Frédéric Buclin]

Probably too invasive for 4.4, so targetted 5.0.