100601 - exclude bugs numbered is horribly inefficient and broken too

Status: NEW

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

Description

[Asa Dotzler [:asa]]

when I tried to "exclude" bugs numbered "<a very big list, maybe about 800 bugs>
and then a boolean tool "attachment data" contains regexp "+++" I get this
error. I don't get the error when I do a "attachment data" contains case
insensitive substring "+++" but then the query seems to time out or die on the
pondering page. So this bug is actually for the problem of my query not
returning results but there is also a problem here that we have a horrible way
of doing the exclude bug numbers. 

Please stand by ...
Content-type: text/html
Software error:
SELECT bugs.bug_id, bugs.groupset, substring(bugs.bug_severity, 1, 3),
substring(bugs.priority, 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.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 LEFT JOIN attachments
attachments_0 ON bugs.bug_id = attachments_0.bug_id WHERE bugs.assigned_to =
map_assigned_to.userid AND bugs.reporter = map_reporter.userid AND bugs.groupset
& 3583 = bugs.groupset AND (NOT (lower(bugs.bug_id) regexp
'(^|[^a-z0-9])959($|[^a-z0-9])' OR lower(bugs.bug_id) regexp
'(^|[^a-z0-9])2436($|[^a-z0-9])' OR lower(bugs.bug_id) regexp
'(^|[^a-z0-9])4248($|[^a-z0-9])' OR lower(bugs.bug_id) regexp
'(^|[^a-z0-9])4830($|[^a-z0-9])' etc, etc for every bug in the exclude.

Comment 1

[Matthew Tuck [:CodeMachine]]

I think there's a URL length limit, maybe you hit it.

Comment 2

[Dave Miller [:justdave]]

while we're at it, something about those regexps just looks WRONG to me.  Bug
IDs are positive integers.  There shouldn't be any other text in the bug_id
column.  WTF is the [a-z0-9] on either side of the bug id for?

Comment 3

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

Yeah, there isn't any need for regexps or anything here, just "bug_id IN (...)"
or "bug_id NOT IN (...)" conditionals.

Comment 4

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 5

[Joel Peshkin]

[e-mail] seems to care about these so they wont make 2.18.  If
someone adopts them, they can move them back. Retargetting to 2.20

Comment 6

[Max Kanat-Alexander]

Wow, this still happens on the tip. This is some really silly SQL. I wonder if I
can fix it right now...

Comment 7

[Max Kanat-Alexander]

Here's the offending code:

>    if ($params->param('bug_id')) {
>        my $type = "anyexact";
>        if ($params->param('bugidtype') && $params->param('bugidtype') eq
'exclude') {
>            $type = "nowords";
>        }
>        push(@specialchart, ["bug_id", $type, join(',',
>$params->param('bug_id'))]);
>    }

Comment 8

[Max Kanat-Alexander]

Attachment: Add a "notanynumber" chart and make Exclude use it

OK, this seems to work nicely. I didn't want to just modify anyexact, because I
didn't want to quote the arguments (more standards-compliant DBs might complain
about it).

Comment 9

[Bradley Baetz (:bbaetz)]

Comment on attachment 176357 [details] [diff] [review]
Add a "notanynumber" chart and make Exclude use it

Why notanynumber? Why not just notany ? Would probably make sense to move some
of the other checks over to that, for things (eg resolutions, etc) that are
distinct items rather than phrases.

Comment 10

[Joel Peshkin]

Comment on attachment 176357 [details] [diff] [review]
Add a "notanynumber" chart and make Exclude use it

This is the wrong place to fix this as it will only help the hard-coded chart
and not the boolean chart.

What you want to do is to change the behavior of anyexact in the general query
construction if the field is "bug_id"

So, you want a funcdef matching....

"^bug_id,anyexact" => sub {
and...	at that point, you may as well just build the sql right there. (unless
you can think of any other use for notanumber)

Comment 11

[Max Kanat-Alexander]

(In reply to comment #9)
> (From update of attachment 176357 [details] [diff] [review] [edit])
> Why notanynumber? Why not just notany?

  Because numbers shouldn't be quoted, while strings should be quoted.

Comment 12

[Max Kanat-Alexander]

(In reply to comment #10)
> What you want to do is to change the behavior of anyexact in the general query
> construction if the field is "bug_id"
> 
> So, you want a funcdef matching....
> 
> "^bug_id,anyexact" => sub {

  Except this is notanyexact, which doesn't exist yet. Also, anyexact quotes its
values, while searching for integers shouldn't. While this is fine in MySQL, I
have no guarantee that other databases will be so friendly as to automatically
cast our varchars to integers.

  It should probably cover anything which could be a number, although I'm not
sure what else in the charts you can search for that would be an integer at this
point. I'd have to look into that.

Comment 13

[Max Kanat-Alexander]

Hey Joel, could you help me out with my last comment?

Comment 14

[Joel Peshkin]

You want to match anyexact and then negate that whole chart.  Otherwise, you'll descend into boolean logic hell.

Comment 15

[Max Kanat-Alexander]

(In reply to comment #14)
> You want to match anyexact and then negate that whole chart.  Otherwise, you'll
> descend into boolean logic hell.

  OK, yeah, I think that's a good idea.

  My only question is: how do I do that? I don't know how to negate something, in Search.pm.

  Right now we have:

if ($params->param('bugidtype') && $params->param('bugidtype') eq 'exclude') {
    $type = "nowords";
}

  I need to figure out how to keep that as anyexact, and then negate it.

Comment 16

[timeless]

it's not possible today. I have a patch *somewhere* that introduces a way to do this. in retrospect, I'm pretty sure it could be done w/o the extra structure I created, however I'll try to get those in.

Comment 17

[Frédéric Buclin]

This ugliness still exists in 4.1. :(

Comment 18

[Max Kanat-Alexander]

(In reply to comment #17)
> This ugliness still exists in 4.1. :(

  Yes it does. It will be possible after the refactoring of boolean charts, or when I add a notanyexact operator.

Comment 19

[Wayne Mery (:wsmwk)]

(In reply to Max Kanat-Alexander from comment #18)
> (In reply to comment #17)
> > This ugliness still exists in 4.1. :(
> 
>   Yes it does. It will be possible a) after the refactoring of boolean charts,
> or b) when I add a notanyexact operator.

"a" is finished?