As part of the Search refactoring of Bugzilla 4.2, all Custom Search clauses were made totally independent. That means that now: "Attachment description" "contains" "foo" AND "Attachment filename" "contains" "bar" Means: "Any bug where one attachment's description contains 'foo' and one attachment's description contains 'bar'." This was done in order to make Custom Search behave totally predictably and extremely simply. This actually resolves many, many support requests and bugs that have been filed over the years about "I added two criteria and weird things happened!" However, it's still important to be able to also say, "I want to find one attachment that matches these two [three, four, n] criteria." or "I want to find one flag that matches all these criteria." In particular, that Flags search feature has been asked for for a long time. This bug tracks adding in a comprehensive functionality for this that will be far better than the hacky, hidden, cryptic functionality that boolean charts *sometimes* implemented for this. My current proposal is that you can specify that you're searching an "attachment" and then specify several criteria for just that attachment. You can search "a bug" and specify several critera for that bug. You can search "a comment" and specify several criteria for that comment. And so on. This functionality won't be available in Bugzilla 4.2, even though this does represent a regression for the people who were familiar with the secret field-associating behaviors of boolean charts. This work could take several months, and I don't want to hold up 4.2 for that reason. However, all people concerned about this should rest assured that we very much *do* want this functionality, and it will be *much better* and *far more functional* than it was in Bugzilla 4.0 when it is completed.

(In reply to Max Kanat-Alexander from comment #0) > Means: "Any bug where one attachment's description contains 'foo' and one > attachment's description contains 'bar'." This should read: "and one attachment's **filename** contains 'bar'."

(From Max Kanat-Alexander on bug 759999 comment #2) > I had a plan for future versions of search to allow you to do criteria that > explicitly depend on each other. For example, for this case the UI would > look something like: > > [ Change ] [ by ] [ is equal to ] [ rc5tv@home.com ] > [ date ] [ is less than ] [ 2008-02-01 ] > [ date ] [ is greater than or equal to ] [ 2008-01-01 ] > > (The other options in the "Change" menu would be "Bug," "Attachment," and > "Comment".) > > Given my current availability, somebody else will have to implement that UI. (From Max Kanat-Alexander on bug 759999 comment #3) > The first step toward implementing that UI would be to implement a backend > that can handle criteria like that, and deciding on how the URL parameters > should be structured (keeping in mind that short URL parameters are > preferred and that it should be backwards-compatible with what is there > now). I'd really appreciate somebody working on this, although it would > certainly be a fair bit of work (at least one entire release cycle for one > engineer).

https://bugzilla.mozilla.org/buglist.cgi?field0-0-0=longdesc;type0-0-0=changedby;value0-0-0=HeroreV%40yahoo.com;field0-1-0=longdesc;type0-1-0=substring;value0-1-0=Components.interfaces Should return only bug 292789 and bug 429070. On the Bugzilla 4.2 staging server, it also returns bug 331259. I do this kind of query almost once a day; losing it (as a regression from bug 638623) would be quite a blow :( Note that filtering out the noise myself isn't an option, because the new query takes too long and times out when the commenter in question is a heavy bmo user like me.

I've created bug 780820 to deal with a subset of this work.

adds the ability to search for changes made to the same field.

(In reply to Byron Jones ‹:glob› from comment #7) > Created attachment 675241 [details] [diff] [review] > patch v1 > > adds the ability to search for changes made to the same field. Following warning from prove xt/search.t: Use of uninitialized value $joiner in string eq at Bugzilla/Search.pm line 1650. at Bugzilla/Search.pm line 1650 Bugzilla::Search::_custom_search() called at Bugzilla/Search.pm line 1590 Bugzilla::Search::_params_to_data_structure() called at Bugzilla/Search.pm line 1572 Bugzilla::Search::_charts() called at Bugzilla/Search.pm line 1559 Bugzilla::Search::_charts_to_conditions() called at Bugzilla/Search.pm line 692 Bugzilla::Search::sql() called at xt/lib/Bugzilla/Test/Search/FieldTest.pm line 533 Test::Exception::lives_ok() called at xt/lib/Bugzilla/Test/Search/FieldTest.pm line 533 Bugzilla::Test::Search::FieldTest::do_tests() called at xt/lib/Bugzilla/Test/Search/FieldTest.pm line 517 Bugzilla::Test::Search::FieldTest::run() called at xt/lib/Bugzilla/Test/Search.pm line 888 Bugzilla::Test::Search::run() called at xt/search.t line 36

addresses undef warnings thrown by xt/ tests

As discussed with glob on IRC, I will be happy to take it for 4.4 if it's reviewed on time and works fine, but I won't block 4.4 on it.

One issue I found when comparing queries on a non-patched trunk with a patched one: http://centos/677757/buglist.cgi?o5=notequals&f1=OP&f0=OP&o2=notequals&f4=OP&v5=0&query_format=advanced&j1=OR&f3=CP&f2=dependson&j4=OR&f5=blocked&f6=CP&v2=0&f7 Without: SELECT bugs.bug_id AS bug_id, bugs.bug_severity AS bug_severity, bugs.priority AS priority, bugs.bug_status AS bug_status, bugs.resolution AS resolution, map_product.name AS product, map_component.name AS component, CASE WHEN INSTR(CAST(map_assigned_to.login_name AS BINARY), CAST('@' AS BINARY)) != 0 THEN SUBSTR(map_assigned_to.login_name, 1, INSTR(CAST(map_assigned_to.login_name AS BINARY), CAST('@' AS BINARY)) - 1) ELSE map_assigned_to.login_name END AS assigned_to, bugs.short_desc AS short_desc, bugs.delta_ts AS changeddate FROM bugs LEFT JOIN bug_group_map AS security_map ON bugs.bug_id = security_map.bug_id INNER JOIN products AS map_product ON bugs.product_id = map_product.id INNER JOIN components AS map_component ON bugs.component_id = map_component.id INNER JOIN profiles AS map_assigned_to ON bugs.assigned_to = map_assigned_to.userid WHERE bugs.creation_ts IS NOT NULL AND security_map.group_id IS NULL AND bugs.bug_id NOT IN (SELECT blocked FROM dependencies WHERE dependson = 0) AND bugs.bug_id NOT IN (SELECT dependson FROM dependencies WHERE blocked = 0) GROUP BY bugs.bug_id ORDER BY bug_id LIMIT 500 Results: > 10000 (hit the internal limit) With: SELECT bugs.bug_id AS bug_id, bugs.bug_severity AS bug_severity, bugs.priority AS priority, bugs.bug_status AS bug_status, bugs.resolution AS resolution, map_product.name AS product, map_component.name AS component, CASE WHEN INSTR(CAST(map_assigned_to.login_name AS BINARY), CAST('@' AS BINARY)) != 0 THEN SUBSTR(map_assigned_to.login_name, 1, INSTR(CAST(map_assigned_to.login_name AS BINARY), CAST('@' AS BINARY)) - 1) ELSE map_assigned_to.login_name END AS assigned_to, bugs.short_desc AS short_desc, bugs.delta_ts AS changeddate FROM bugs LEFT JOIN bug_group_map AS security_map ON bugs.bug_id = security_map.bug_id INNER JOIN products AS map_product ON bugs.product_id = map_product.id INNER JOIN components AS map_component ON bugs.component_id = map_component.id INNER JOIN profiles AS map_assigned_to ON bugs.assigned_to = map_assigned_to.userid WHERE bugs.creation_ts IS NOT NULL AND security_map.group_id IS NULL AND 1=2 AND 1=2 GROUP BY bugs.bug_id ORDER BY bug_id LIMIT 500; Results: 0 The results should be the same. Strangely "AND 1=2 AND 1=2" is being added instead of the clauses regarding blocked/dependson so the results will always be 0. dkl

I see what is happening from comment 11: sub build_subselect { - my ($outer, $inner, $table, $cond) = @_; - return "$outer IN (SELECT $inner FROM $table WHERE $cond)"; + my ($outer, $inner, $table, $cond, $negate) = @_; + # Execute subselects immediately to avoid dependent subqueries, which are + # large performance hits on MySql + my $q = "SELECT $inner FROM $table WHERE $cond"; + my $dbh = Bugzilla->dbh; + my $list = $dbh->selectcol_arrayref($q); + return "1=2" unless @$list; + return $dbh->sql_in($outer, $list, $negate); } If the condition is negated "NOT IN(...)" maybe we should return empty string if !@$list return if (!@$list && $negate); dkl

Comment on attachment 675856 [details] [diff] [review] patch v2 Review of attachment 675856 [details] [diff] [review]: ----------------------------------------------------------------- Except for the minor issue mentioned in comment 11, this looks good to me and works with all my other testing executed. the other nits such as the addition of extra rows on <back> and the inability to remove rows are not regressions and can be fixed in other bugs. r=dkl

Err.... wait! Comment 0 is about joining several criteria against the same object, such as attachments: "I want to find one attachment that matches these two [three, four, n] criteria." So for instance, I want the following: Attachment creator contains bugzilla AND Attachment MIME type is equal to text/plain Without your patch, they behave as described in comment 0, i.e. not bound to the same attachment. With your patch, it behaves exactly the same way, so there is no improvement. If I select "against the same field", it forces me to choose between Attachment creator and Attachment MIME type, but I cannot select both, i.e. it's really bound to the same field but not to the same object, which is too restrictive. I see that glob reworded the bug summary to replace bug/attachment by field, which is only a subselect of the problem described here. This is not what this bug is about. Please reopen bug 780820 (whose bug summary must be adapted to be less restrictive) and attach your patch there. But this bug is really about what comment 0 describes.

(In reply to David Lawrence [:dkl] from comment #12) > If the condition is negated "NOT IN(...)" maybe we should return empty > string if !@$list > > return if (!@$list && $negate); Don't do that. If you do this change, then Bugzilla crashes if it doesn't match any bug.

Comment on attachment 675856 [details] [diff] [review] patch v2 >=== modified file 'Bugzilla/Search.pm' > sub build_subselect { >- my ($outer, $inner, $table, $cond) = @_; >- return "$outer IN (SELECT $inner FROM $table WHERE $cond)"; >+ my ($outer, $inner, $table, $cond, $negate) = @_; >+ # Execute subselects immediately to avoid dependent subqueries, which are >+ # large performance hits on MySql >+ my $q = "SELECT $inner FROM $table WHERE $cond"; >+ my $dbh = Bugzilla->dbh; >+ my $list = $dbh->selectcol_arrayref($q); >+ return "1=2" unless @$list; >+ return $dbh->sql_in($outer, $list, $negate); > } I don't like this subroutine. It generates an incredibly long list of bug IDs in the SQL query. Just append &debug=1 to the URL and see the output. I hope MySQL or any other DB server is clever enough to optimize subselects without having to pass a list with several thousands of items. Do you have data about what you argue here? Moreover, the generated list has the same bug IDs several tens of times. Looks like a DISTINCT is missing in the SQL query. This must be fixed. As dkl's fix in comment 12 is incorrect (Bugzilla crashes with it), it looks like the problem described in comment 11 needs a new patch rather than a fix on checkin.

(In reply to Frédéric Buclin from comment #16) > I don't like this subroutine. It generates an incredibly long list of bug > IDs in the SQL query. Just append &debug=1 to the URL and see the output. I > hope MySQL or any other DB server is clever enough to optimize subselects > without having to pass a list with several thousands of items. Do you have > data about what you argue here? yes, mysql isn't clever enough to optimise subselects; instead it will execute the subselect **for every row** even if it is lacking dependencies. same query in 4.0 takes 7 seconds, 4.2 takes 223 seconds. i don't think "not liking the long list of bug ids in the sql query" is a valid reason for rejecting this change, as that's purely hidden cosmetics. > Moreover, the generated list has the same bug IDs several tens of times. > Looks like a DISTINCT is missing in the SQL query. This must be fixed. As > dkl's fix in comment 12 is incorrect (Bugzilla crashes with it), it looks > like the problem described in comment 11 needs a new patch rather than a fix > on checkin. can do. will move back to bug 780820 and morph.

(In reply to Frédéric Buclin from comment #16) > This must be fixed. As > dkl's fix in comment 12 is incorrect (Bugzilla crashes with it), it looks > like the problem described in comment 11 needs a new patch rather than a fix > on checkin. Yeah I see that what should have been returned now is "1=1" and was just a possible idea that was completely untested. dkl

(In reply to Byron Jones ‹:glob› from comment #17) > same query in 4.0 takes 7 seconds, 4.2 takes 223 seconds. Which example did you use? I'm interested to do some comparisons myself. > i don't think "not liking the long list of bug ids in the sql query" is a > valid reason for rejecting this change, as that's purely hidden cosmetics. That's not the reason of my r-. I denied review because dkl found a bug in your patch, and his proposed fixed made Bugzilla to crash. This is why I said a new patch was needed.