300709 - Avoid the use of SELECT *

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Frédéric Buclin]

We have no idea what is returned by " SELECT * " without looking at the DB
itself, especially the order of the columns. When new columns are added by
checksetup.pl due to an upgrade, the order of these columns may be different
from the order obtained for a new installation which uses Bugzilla/DB/Schema.pm.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[GavinS]

Should we also have a runtests test for 'SELECT *' as well?

Comment 3

[:glob ✱]

Comment on attachment 189252 [details] [diff] [review]
patch, v1

r=glob

i agree with GavinS, bonus points for a test.

Comment 4

[Frédéric Buclin]

(In reply to comment #2)
> Should we also have a runtests test for 'SELECT *' as well?

No need. It's the job of the reviewer to forbid such things IMO.

Comment 5

[Max Kanat-Alexander]

(In reply to comment #4)
> No need. It's the job of the reviewer to forbid such things IMO.

  Yeah, just like unfiltered directives and tabs in code files! And some common
mispellings, too. In fact, who needs runtests at all, when you have reviewers? :-D

/me only did this because he knows LpSolit has a good sense of humor. :-)

Comment 6

[Dave Miller [:justdave]]

Bonus points for backporting the collectstats chunk of this to 2.18 and 2.16.
(and freaking test it! :)

I don't know how this ever snuck in, but that's darn unreliable, and in code
that's collecting statistics even.  Nothing like inverted stats, eh?

Comment 7

[Dave Miller [:justdave]]

The 2.16 version should use SendSQL of course...

Comment 8

[Frédéric Buclin]

Attachment: 2.18 backport, v1

collectstats.pl is the single file to contain a "SELECT *" in 2.18.3.

Comment 9

[Frédéric Buclin]

Attachment: 2.16 backport, v1

Tested on 2.16.10. Works!

Comment 10

[:glob ✱]

Comment on attachment 189604 [details] [diff] [review]
2.18 backport, v1

r=glob by inspection

Comment 11

[:glob ✱]

Comment on attachment 189606 [details] [diff] [review]
2.16 backport, v1

r=glob by inspection

Comment 12

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

Have you tested this in 2.18 and 2.20 as well?

Comment 13

[Frédéric Buclin]

tip:

Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.44; previous revision: 1.43
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.64; previous revision: 1.63
done


2.20rc1:

Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.43.4.1; previous revision: 1.43
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.61.2.3; previous revision: 1.61.2.2
done


2.18.3:

Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.38.2.3; previous revision: 1.38.2.2
done


2.16.10:

Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.20.12.3; previous revision: 1.20.12.2
done