104682 - SQL objects

Status: VERIFIED WONTFIX

(Bugzilla :: Database, enhancement, P4)

Description

[Matthew Tuck [:CodeMachine]]

We should introduce reusable SQL objects into the system.  By this I mean the
object has members called tables, fields, where-conditions, etc.  We would use
lists to separate distinct entities, rather than commas.  The object would get
collapsed to a string when sent to SendSQL.

The advantage of this is that it is easier to modify these than strings, and
probably faster too when you do so.

See bug #97469 for the grotty search and replace code that inspired this RFE. 
buglist.cgi would probably benefit from this as well.

Comment 1

[Bradley Baetz (:bbaetz)]

I tried hacking this into my SelectSQL stuff (making it take a set of paramaters
for the various parts of the SQL clause), but it just got confusing. Note that
we still have to do search and replace - ie I can't add the cc to the list of
tables if its already there.

Comment 2

[Dave Miller [:justdave]]

If you've looked at buglist.cgi you might notice it already does this (makes
lists of each of the individual parts of the query, then tacks them all together
in a flat string before passing it to SendSQL).

Comment 3

[Matthew Tuck [:CodeMachine]]

Yes I've seen something similar, but we should make it reusable and move it to
globals.

Comment 4

[Paul Baker]

I would also recommend making it so bugzilla can handle multiple sql statements
at once instead of one.

It would also make sense to take advantage of full power DBI using things like
placeholders and stored statement handles. Perhaps that should go in as a new bug?

Comment 5

[Matthew Tuck [:CodeMachine]]

Yes, that would be extra bugs.

Placeholders would definitely be a good idea, we should probably evangelise
their existence and work out what should be converted.

By stored statement handles do you mean stored Perl procedures?  I'm not exactly
sure what you mean by that.  In either case, I'd like a quick explanation of the
benefits of this feature and where it might be used.

Comment 6

[Matthew Tuck [:CodeMachine]]

Oh, and regarding multiple statements, do you mean transactions?  I don't really
see a benefit of having multiple statement taking otherwise.  It's simply two
calls in the rare case.

If you are talking about transactions, we'll likely look at them once we have DB
support for PostgreSQL which is meant to support them well.

Comment 7

[Paul Baker]

> By stored statement handles do you mean stored Perl procedures?

No, I simply mean that when we should try to create dbi statement handles ($sth)
that can be reused. I haven't ever really looked at any of the SQL code that is
used in bugzilla, but for instance if we are doing something like "SELECT * FROM
bugs WHERE id = #bugid" we can instead do something like this using DBI.

my $select_bug_sth = $dbh->prepare('SELECT * FROM bugs WHERE id = ?');

Then we can do $select_bug_sth->execute($bug_id_we_are_looking_for) as many
times as we want, and mysql will never have to reparse the sql for that
statement. That will give speed increases.

if there are any other sql selects that we do more than once, we can create a
dbi statement handles for them and just keep reusing them instead of always
having to piece together SQL over and over again and calling SendSQL

Further, there are also cached statements in DBI, but you have to be more
careful using those, but you will get even bigger performance wins using those.

> Oh, and regarding multiple statements, do you mean transactions?  I don't
> really see a benefit of having multiple statement taking otherwise.

Basically what I mean is having several select statements going at once. For
instance in showdependencytree.cgi there is the one main SendSQL call that then
quickly grabs everything from that and dumps it all into this big array. Then
starts to iterate through that array. You shouldn't have to do that. You should
just iterate through each fetch one at a time and be able to make more sql calls
as needed without it screwing up your original fetch.

It's nice to have the wrappers around DBI to kinda dump down things, but it ends
up making more advanced things A LOT harder to do. Perl is meant to make easy
things easy without making hard things impossible. To me the SQL wrappers don't
achieve that goal. DBI gave perl an extremely powerful yet easy to use OO
interface to databases, and the procedural SQL wrappers used in Bugzilla kinda
defeat that. Just my opinion, based on my initial impressions and experiences
trying to modify my companies bugzilla installation. I could be wrong. Just that
if one sticks with the SQL wrappers you have to do a lot of dancing around to do
something that is rather trivial just using straight away DBI (like iterating
through more than one select at a time)

Look at bug #101164 for a good reason while multiple selects are needed. Matt
(the reporter) even says that a solution for it would be to "use separate
queries against each of these tables.  Manually join the data together...However
the Bugzilla code base doesn't currently support two queries being read
at once.  I disagree with this, I've wanted to do it before." Which is what
caused me to do a search for a bug related to adding that ability and what I
found was this bug. How I ended up here is because what I really want is to have
a dependency list column in the buglist (bug#102048).

Comment 8

[Matthew Tuck [:CodeMachine]]

As you can see I certainly feel multiple statements at once are valuable, but I
don't think they're that hard to implement in the current scheme, simply add a
return value to SendSql or some such, which can be kept.  I think the wrappers
are valuable.

You can currently use PushSQLState and PopSQLState to do some distinct
processing for each record in an initial query, although I'm not sure of the
performance aspects of this (I imagine there is very little impact).

Comment 9

[Dave Miller [:justdave]]

bbaetz: this bug is really old.  how does this mesh with your current doings?

Comment 10

[Bradley Baetz (:bbaetz)]

justdave: Umm. Well, see Bug.pm, I guess. Thats readonly, but....

This bug is more for a DBIx::Simple (I think that what its called) approach, but
I don't think that works for us, given the complex interactions + stuff we ahve
to do in perl, rather than via stored procedures.

Comment 11

[Dave Miller [:justdave]]

Reassigning all of my "future" targetted bugs to indicate that I'm not presently
working on them, and someone else could feel free to work on them.

Comment 12

[Dave Miller [:justdave]]

Reassigning all of my "future" targetted bugs to indicate that I'm not presently
working on them, and someone else could feel free to work on them. (sorry for
the spam if you got this twice, it didn't take right the first time)

Comment 13

[Max Kanat-Alexander]

OK, so this is probably now a duplicate of either bug 278017 or bug 122922.

Or, per comment 7, since we did move to using the DBI stuff, we could just
WONTFIX this and decide that it's better to use the DBI stuff directly and never
use SQL objects.

justdave?

Comment 14

[Frédéric Buclin]

(04:23:02) myk: LpSolit: seems like a closed wontfix per comment 10
(04:25:30) LpSolit: myk: ok for a WONTFIX
(04:28:35) myk: LpSolit: ok by me

Comment 15

[Max Kanat-Alexander]

We might use something like this for Search.pm, but we'd never use it across all
of Bugzilla unless it was extremely performant (which is unlikely to ever happen).