oc-shopaholic-plugin icon indicating copy to clipboard operation
oc-shopaholic-plugin copied to clipboard

Slow loading ProductList (attachments )

Open kdzone opened this issue 5 years ago • 7 comments

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?

kdzone avatar Feb 22 '20 21:02 kdzone

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

kharanenka avatar Feb 24 '20 10:02 kharanenka

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

fosterushka avatar Mar 22 '21 11:03 fosterushka

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.

LukeTowers avatar Mar 22 '21 15:03 LukeTowers

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

kdzone avatar Mar 22 '21 16:03 kdzone

This type match this field at 99.9999%

kdzone avatar Mar 22 '21 16:03 kdzone

@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.

LukeTowers avatar Mar 22 '21 17:03 LukeTowers

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

daftspunk avatar Mar 22 '21 21:03 daftspunk