From system/library/cart.php file, I noticed an exaggerated use of SQL SELECT queries since all those query types are under PHP loops and it is obviously not recommended.
I spent half the day to recode this class file and hopefully it will help others by optimizing their SQL resources in the future when customers adds / edit their products from OpenCart. I uploaded two files in order to make the proper corrections for your store. Due to the size of text that can't be posted on the forum since we're only allowed to post a maximum of 20000 characters, you can use Notepad++; to compare two files. The codes is for OpenCart v1.5.13 and probably above if not fixed soon enough on the next releases.
First one for: system/library/cart.php file.
Second one for: catalog/controller/checkout/cart.php file.
Note: Integrate this solution on a test site folder before implementing into your live store.
Edit: I had to remove the system/library/cart.php file due to odd behavior I encountered regarding the total price calculation built from the original file. A similar post has been posted here: http://forum.opencart.com/viewtopic.php?f=161&t=46719
Attachments
Missing product_id key and value once gathered from the cart class file in the system library.
Last edited by straightlight on Thu Nov 24, 2011 8:42 am, edited 13 times in total.
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
Interesting. I'll have to find some time to go over it more thoroughly. A few things from a quick skimming:
1) You definitely want to make sure to sanitize the product_ids in your queries.
2) gettype is defunct. You'll want to use is_array instead.
3) Super minor - stylistically OpenCart uses implode instead of join
1) You definitely want to make sure to sanitize the product_ids in your queries.
2) gettype is defunct. You'll want to use is_array instead.
3) Super minor - stylistically OpenCart uses implode instead of join
-Ryan
The product ID from the $this->session->data['cart'] => $product[0] is already sanitized from the $product_id during the first loop. It wouldn't be quite useful to re-sanitize what has already been sanitized during the first loop while depending on the same array while during an SQL query or PHP validations.1) You definitely want to make sure to sanitize the product_ids in your queries.
Done. I just found out gettype is useful for objects and not to distinguish presence of an array. I will post an update in a minute.2) gettype is defunct. You'll want to use is_array instead.
Edit: File updated.
False. If there's a single value in an array, and implode is being used, an error message gets returned from PHP v5.3+ (during SQL queries). Join is the only function which does not return an error even though a single value from the array is being found. Implode starts from 2 values. However, I do not know logically if this is a PHP bug but this is what I did noticed while comparing the twos from most of my PHP development.3) Super minor - stylistically OpenCart uses implode instead of join
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
system/library/cart.php file re-updated for sanitizing the product options though.
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
Well, his statement is not false (stylistically, Daniel does use implode instead of join), but that's interesting information about implode vs. join.straightlight wrote:False. If there's a single value in an array, and implode is being used, an error message gets returned from PHP v5.3+ (during SQL queries). Join is the only function which does not return an error even though a single value from the array is being found. Implode starts from 2 values. However, I do not know logically if this is a PHP bug but this is what I did noticed while comparing the twos from most of my PHP development.3) Super minor - stylistically OpenCart uses implode instead of join
However, the documentation for join says it's just an alias for implode, and I can't reproduce this in a simple test. Can you give an example of where you see an error with implode and not join?
An example you can use is from the PDO Extension (MySQL) I coded in the contribution if you do have access locally from OpenCart server to download it. You can see the PDO's construction file which only contains joins to make the preparations of any SQL queries involving to write into the database. While using implode, you should and probably will see an error message appearing due to single values while using join; it won't.
Regarding the alias from the PHP manual, I do agree but as stated above, without knowing exactly if it's a PHP bug, implode will not return the expected results if only a single index exists from an array when querying from SQL (IN / NOT IN).
Regarding the alias from the PHP manual, I do agree but as stated above, without knowing exactly if it's a PHP bug, implode will not return the expected results if only a single index exists from an array when querying from SQL (IN / NOT IN).
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
As a single item from your basket or multiple items ? Can you confirm if it works on both ?dony_b wrote:works with 1.5.1.2
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
This SQL optimization actually prevents SELECT statement from SQL to be looped constantly by each $this->session->data['cart'] => $key and not recommended to loop SELECT statements especially since the original codes contains multi-dimensional loops in the class. This alternative approach I coded will simply make the call for each singled SELECT and be looped from PHP when returned to the SQL object and associated accordingly by each product ID which mainly matches with the basket's product ID - including the product options, option values, points and so on.
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
It's strange I haven't experienced the issue you are describing with implode and I use it quite frequently to pass data to mysql IN and NOT IN functions.
I'm certainly not against this type of optimization, but out of curiosity why start with the cart? Seems like there are other places that would benefit more from it as the cart generally has a rather limited amount of data in it.
I'm certainly not against this type of optimization, but out of curiosity why start with the cart? Seems like there are other places that would benefit more from it as the cart generally has a rather limited amount of data in it.
OpenCart commercial mods and development http://spotonsolutions.net
Layered Navigation
Shipment Tracking
Vehicle Year/Make/Model Filter
We learn something new everyday.Xsecrets wrote:It's strange I haven't experienced the issue you are describing with implode and I use it quite frequently to pass data to mysql IN and NOT IN functions.
I'm certainly not against this type of optimization, but out of curiosity why start with the cart? Seems like there are other places that would benefit more from it as the cart generally has a rather limited amount of data in it.
Regarding the curiosity on why start with the cart? A simple answer would be to state that it wouldn't be quite idealistic to post all the subjects at the same time while I do agree there might be more important place in OpenCart to be revised as such. Besides, the forum has a maximum characters in posting so it wouldn't be possible to post all ideas at the same time. The cart class was simply the first place I started re-coding. However, I do am tracking right now for other locations.
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
Join is implode.straightlight wrote:If there's a single value in an array, and implode is being used, an error message gets returned from PHP v5.3+ (during SQL queries).
http://php.net/manual/en/function.join.php
-Ryan
rph wrote:Join is implode.straightlight wrote:If there's a single value in an array, and implode is being used, an error message gets returned from PHP v5.3+ (during SQL queries).
http://php.net/manual/en/function.join.php
straightlight wrote: Regarding the alias from the PHP manual, I do agree but as stated above, without knowing exactly if it's a PHP bug, implode will not return the expected results if only a single index exists from an array when querying from SQL (IN / NOT IN).
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
Yea I found the same... in fact implode is a tad faster:
http://blog.tryangled.com/2008/07/15/im ... in-in-php/
http://blog.tryangled.com/2008/07/15/im ... in-in-php/
The problematic here as explained isn't about the speed but I sure did found your posted URL interesting. However, I made some research this morning and the problem I'm facing with implode vs join might be because of this explanation:Qphoria wrote:Yea I found the same... in fact implode is a tad faster:
http://blog.tryangled.com/2008/07/15/im ... in-in-php/
http://forum.codecall.net/php-tutorials ... split.html
While using preg_split, what it doesn't describe issues with implode while trying to re-extract an array from a loop. I then tested this on one of my modules I used to saw an error with implode and this is exactly why. If using 'explode' rather than preg_split, the logic is a bit differed from one and another which is why bigger chances to see an implode error message. Meaning, the alias function people might think it is between join and implode ain't what it seem to be and depending from what it's being filled previously in order to re-extract the values, the results are differed for unexplained reasons; perhaps due to the wrong switch cases from preg_split that haven't been used properly before implying the implode command. I will have to investigate more on this
During while, I will make the corrections from join to implode from my first post since preg_split is not involved in this constructor.
Edit: File updated.
Dedication and passion goes to those who are able to push and merge a project.
Regards,
Straightlight
Programmer / Opencart Tester
I can't seem to reproduce his results. Does he give his methodology anywhere?Qphoria wrote:Yea I found the same... in fact implode is a tad faster:
http://blog.tryangled.com/2008/07/15/im ... in-in-php/
-Ryan
I don't see anything relevant in that link. I looked in the PHP 5.3.8 source and join literally is implode.straightlight wrote:The problematic here as explained isn't about the speed but I sure did found your posted URL interesting. However, I made some research this morning and the problem I'm facing with implode vs join might be because of this explanation:
http://forum.codecall.net/php-tutorials ... split.html
-Ryan
Who is online
Users browsing this forum: No registered users and 45 guests
