oc-shopaholic / oc-shopaholic-plugin

🛍️ No. 1 e-commerce platform for October CMS
https://shopaholic.one
GNU General Public License v3.0
425 stars 52 forks source link

Slow loading ProductList (attachments ) #253

Open kdzone opened 4 years ago

kdzone commented 4 years ago

I noticed that when paginate the list of product (before caching), the speed dropped to 4-5 seconds. After analysis, I found an interesting defect. When loading a product, SQL executed:

select
*
from
system_files
where
`system_files`.`attachment_type` = 'Lovata\Shopaholic\Models\Product'
and `system_files`.`attachment_id` = 3785
and `field`=  'preview_image'
and `system_files`.`attachment_id` is not null
order by
`sort_order` asc limit 1

The request is great, but 3785 is a number, and system_files.attachment_id is varchar (191). The data types are different! In this case, MySQL does not use the index on the attachment_id field. Here is an explanation...

Product model has preview_image and images file attachments.

With a large number of records in system_files, speed drops dramatically.

I do not know a general solution to this problem. This is more of a CMS problem. OctoberCMS developers everywhere recommend writing:

public $attachOne = [
    'field_name' = 'System\Models\ File'
];

But if the key field of model is not varchar, the index on the attachment_id field will not work.

In my database, I changed the type of the attachment_id field to int and everything began to work quickly. But this is hard hack.

Maybe there are other solutions?

kharanenka commented 4 years ago

Hi! You are right that issue is problem OctoberCMS. Unfortunately, we cannot fix it. You must try to speak with core team. We are ready to support you when talking with the core team

fosterushka commented 3 years ago

@kdzone Hello there, have you fixed may be some hardcode or have you contact with a core team any updates ?:

LukeTowers commented 3 years ago

The solution would probably be to make the query generators take keyType into account when type casting prepared data so that MySQL is passed '3785' instead of 3785. See https://github.com/wintercms/storm/commit/026d4bb511f4b4ce4162363937f1d28c9af359f8. Feel free to make an issue on wintercms/winter and we can take a look.

kdzone commented 3 years ago

I changed the type of the field attachemnt_id to int(10) unsigned. Harcode.

kdzone commented 3 years ago

This type match this field at 99.9999%

LukeTowers commented 3 years ago

@kdzone the reason the field is varchar to begin with is to support relationships with records that have string keys, so if you know that won't happen for your specific project then it's fine, but I would recommend documenting that change with your project so that any developers (current or future) on the project are aware of that difference.

daftspunk commented 3 years ago

Something else to try is rolling back the fix Luke mentioned above to see if it makes any difference. We plan on implementing a more robust solution for this soon. Thanks for your patience and for using October CMS