Some Bugzilla queries contain line breaks, and this makes server tools like "mysqladmin --verbose processlist" harder to use. Bugzilla queries, even when broken into multiple lines in the Perl code that defines them, should contain no line breaks.

This refers to SQL in the source code right? Why does it make mysqladmin harder to use?

>This refers to SQL in the source code right? Right. >Why does it make mysqladmin harder to use? "mysqladmin processlist" displays a list of queries with one query per line. Having line breaks in the queries messes up this formatting, making it harder for humans and scripts to parse the output.

These unloved bugs have been sitting untouched since June 2002 or longer. If nobody does anything else to them, they certainly won't make 2.18 Retargetting to 2.20. If you really plan to push them right now, you might pull them back in.

Having the line breaks makes things infinitely more readable in the Perl source code.... With our new architecture changes, since I believe Bugzilla::DB is (or shortly will be) overriding DBI, perhaps we could override ->prepare and ->do and friends and collapse whitespace in the queries before passing them to the server? (Or does DBI perhaps already have an option for that?)

Reassigning bugs that I'm not actively working on to the default component owner in order to try to make some sanity out of my personal buglist. This doesn't mean the bug isn't being dealt with, just that I'm not the one doing it. If you are dealing with this bug, please assign it to yourself.

This overrides the following DBI methods in Bugzilla/DB.pm: prepare, do, selectrow_array, selectrow_arrayref, selectrow_hashref, selectall_arrayref, selectall_hashref, selectcol_arrayref This removes all \n chars from the query, so that the processlist output will be the same. It does *not* kill any extra whitespace though, since that may affect the paramaters for the query if they aren't being called through binding them - this is a safety precaution more than anything.

As above, but with the second parameter to prepare

Comment on attachment 191791 [details] [diff] [review] Patch v1.1 (patch -p2 when applying) OK, instead of doing the arguments the way you're doing them, you can do this: my $self = shift; my $query = shift; $query =~ s/\n/ /g; return $self->SUPER::function($query, @_); That allows us to be compatible if the arguments ever change for those functions. Note also that I think it's better to replace the newline with a space than with nothing at all, to be safe.

Addresses review comments

Comment on attachment 191794 [details] [diff] [review] Patch v2 Yep. Seems to work. However, please move all these subs into their own section, instead of where they are now. Put it at the bottom, or something.

It's up to justdave whether or not we actually want to do this. I *think* it wouldn't hurt... my only mild concern is that it is a bit of a strange side-effect to add to these methods.

Comment on attachment 191794 [details] [diff] [review] Patch v2 As much as I like the idea of doing this, we can't just arbitrarily remove all line breaks. Any line breaks that ocurr inside quotes need to be preserved, as that's actual data and not part of the query syntax.

Oh, and also, I just remembered -- the first argument of the select* or do methods can be an $sth, so we have to make sure that it's not.

Only prepare method needs to be overridden because it's called by the select* and do methods. This way there is no need to worry about statement handle parameters. There seems to be code in DBD::Chart's prepare method to remove newlines (since v0.21). Looks awfully complicated but that's probably because it does the preparing part too. Maybe it can give some clues on how to implement this or not..

This isn't worth the required complexity.