woocommerce / action-scheduler-custom-tables

This plugin is no longer needed. The custom table code is now part of Action Scheduler version 3.0 and newer. If you're using the latest Action Scheduler, you have the most performant schema available.
https://actionscheduler.org
14 stars 2 forks source link

Hook length and InnoDB index length limits with utf8mb4 #39

Closed thenbrent closed 6 years ago

thenbrent commented 6 years ago

In a similar vein to #36, while we're changing the DB structure, we may wish to enforce a 191 char limit for the hook column to ensure hooks can be indexed in their entirety.

IMO, 191 is still more than enough chars for a hook, arguably too many really. The only reason it's set to 255 atm is because of the legacy from the post store. So it should be OK and reasonable to change (either to 191, or even lower).

Any objections?

If we decide to reduce the length', we'll also need a similar warning in AS 2.1.0 like the one added with https://github.com/Prospress/action-scheduler/pull/187.

leewillis77 commented 6 years ago

Hi,

I'm following these issues as a potential user of action scheduler, although i don't have a concrete implementation right now.

One thing I did want to note though is that there's nothing stopping you having an indexed varchar(255) on InnoDB with utf8mb4, but the index must be limited to 191.

Effectively this means that only the first 191 chars will be used to create the index.

See 'Column Prefix Key Parts' on https://dev.mysql.com/doc/refman/8.0/en/create-index.html#create-index-column-prefixes

So, I believe that you could leave the column as-is, and just add an index on the first 191 characters which should retain the flexibility of longer hook/args columns and give you the performance improvement you're looking for.

thenbrent commented 6 years ago

Thanks for jumping in @leewillis77. We're aware the restriction only applies to the index.

The intention for changing the column length is to enforce a performant value, rather than continuing to allow developers to use non-performant values (likely without even realising it). As we're still at v1.0 of these schemas, now is the best time to enforce that restriction.

The end game is that once these tables are the default store, Action Scheduler will become performant by default, rather than continuing to allow non-indexable column values, meaning it's only performant as long as the developer uses it right. That will be increasingly important once it starts to get used via WC (where developers may not even be aware they're using AS, and certainly won't be reading docs on best practice).

retain the flexibility of longer hook

The length discussion for the hook column is sort of moot. 255 chars, and even 191, is excessively long and really shouldn't be used.

Here's an example of a 191 char hook: ⁉️

woocommerce_scheduled_subscription_payment_woocommerce_scheduled_subscription_payment_woocommerce_scheduled_subscription_payment_woocommerce_scheduled_subscription_payment_woocommerce_schedul

The question of column length for args is more open, especially given it's coming from a longtext column, and used rarely for search.