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.
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.
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).
Yes I've seen something similar, but we should make it reusable and move it to globals.
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?
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.
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.
> 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).
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).
bbaetz: this bug is really old. how does this mesh with your current doings?
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.
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.
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)
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?
(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
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).