Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Query Builder Efficiency #2456

Closed
ragboyjr opened this Issue · 5 comments

3 participants

ragboyjr Eric "Aken" Roberts Andrey Andreev
ragboyjr

There are some parts in the new query builder that seem to be a lot of overhead for something that doesn't seem to need all of that overhead.

For example, on select(), why do you guys need to explode all of the string values into an array and then implode them again? There also seems to be a lot of validation in the query builder code, which can be very helpful for the development environment, but I can't think of a reason why there needs to be that extra overhead on production environments. I may be missing something though, so if there are concrete reasons why this extra processing needs to be done, I'm all ears. But I feel like Query Builder is a feature that is so critical in CI 3.0 and will be used a lot. So we probably should to try to make this code as fast & memory efficient as possible (I know, I know, who cares about memory, but moving a lot of memory around in php takes longer than moving a little bit of memory around).

In fact, if you think about CodeIgniter in relation to every other php framework out there, it's lacking features in the Database arena e.g. most other frameworks offer an ORM, and have more db helper functions. But if we could market the fact that CI's query builder has been streamlined to make sure that your CI application will have a light footprint and incredible speed compared to other monolithic application, then we have a selling point that'll make CI's db arena as relevant to other php frameworks.

IMHO, the database drivers and libraries a framework offers are one of the most essential items a framework can offer because any web application worth anything will use persistent storage.

Thanks,

RJ

Eric "Aken" Roberts

Items in the SELECT statement are run through to check if a table prefix needs to be added, back ticks and escape characters are added, etc.

Improving the query builder is a nice thought, but just saying that it needs to be done isn't going to make anything happen. You have to be proactive with things like this. If anything, quote specific chunks of code that you're unsure about, or you think can be improved. Or, even better, submit some improvements yourself.

ragboyjr

That's exactly what I wanted to hear. I just wanted to see how modifications to query builder would be accepted. I didn't want to start writing code and then get shut down for whatever reasons. Thanks for the prompt feedback

Eric "Aken" Roberts

I doubt anybody would be against improving the query builder, if the submitted code was actually an improvement (which is useful for more than just a select few users, naturally). Discussion and ideas are encouraged (at least I encourage it -- I'm not a representative for this repo).

If you do plan on submitting code, though, make sure to do it in chunks. Don't radically modify many parts of the query builder and submit it all at once. It's far too difficult to review and test. Focus on one thing and make it as good as you can.

ragboyjr

K, I've been doing some profiling on my machine, and I've come up with two quick fixes that should help some.
1. Pass data by reference instead of returning it for all the internal functions.
2. Use stings instead of arrays for qb_select, qb_from, qb_where, etc.
String concatenation is faster than adding values to arrays and then imploding them. The only problem with using strings vs using the array, is that the error checking / validation would be more difficult in certain functions; however, I feel like some of that validation is superfluous especially on a production environment. For example, in Qb::select(), we split the string into an array just to trim the individual values in the array, and then append each value into a different array. So that's the kind of processing that I feel is superfluous and not needed.

With that said though, there are functions like Qb::from that split a string up into an array for tracking the aliases, which may be useful. So this is probably extra processing that could be helpful; however, it wouldn't be helpful on live. So what I'd like to do is add a boolean flag into the database config of is_production and depending on that flag, we'd do the extra processing for these queries. Is that reasonable?

Andrey Andreev
Owner

The thing is, EVERYBODY has an issue with the QB - some want to write from() prior to select(), others want to pass pure strings and complain that a field name is not escaped and some guy even complained that limit() has problems with concatenated strings under MSSQL 2008 (which btw, DOESN'T SUPPORT LIMIT yet we made it).

You don't get such features without sacrificing some performance and especially not by working exclusively with strings. It might be possible if you're a God-like regexp ninja, but it sure doesn't come easy even then. There's no use of discussing this - it's one of the things that if are to be done, somebody must just go ahead and do them ALL, end-to-end. I'm a performance-oriented guy myself, so don't worry about whether we want it to be faster or not - of course we do. :)

Andrey Andreev narfbg closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.