32 chars too small for rules names?

Just lost 6 hours trying to track this in an install profile :-S

http://drupalcode.org/project/rules.git/commitdiff/db1c6758eb0f31ab0de7f...

When you shorten column lengths in hook_schema()s, you have to also add hook_update_N()s with db_change_field() with the new spec.

Quick patch to 128.

oops, failed to actually grab the patch

Adding a p0 patch for drush make.

This is a fundamental #fail: Can't install at all.

@EclipseGC, Drush Make now supports normal patches. Fix went in 10 days ago or so (finally).

Added more analysis of the error message.

Updated issue summary with more detail.

We probably need to just go back to 255 characters, unless there's some reason not to.

If this is going to be shortened, it needs to have a db_change_field() and also the index has to be removed and rebuilt in hook_update_N().

ouch, sry for creating those troubles.

The UI has already #maxlength == 32 so I thought this is a safe change. Obviously it isn't... :(
I see that programmatically created rules don't have to follow that though, in particular as we don't support name changes there anyway. Anyway, we should sync both values.

Problem is, we also need an index in the mysql rules_scheduler table, but mysql is by default limited to 1000byte indexes and it looks like db-tng doesn't support changing that. With utf8, we have varcharsize*3 for the index, what means with the index (machine_name, identifier) we cannot have 255*3+255*3 (>1000).

I think it doesn't make much sense to have that long machine-names, as it just makes identifying slower/harder. 32 should be basically enough, but I see might be a bit less in certain operations. So what about 64?

64*3+255*3 would work.

What's the longest machine-readable name in drupal commerce? Any reason to have such long names there?

>If this is going to be shortened, it needs to have a db_change_field() and also the index has to be removed and rebuilt in hook_update_N().

That's painful for such a "small change", but yes we need to that.

Thanks, fago. I suspect 64 would be OK. (Note that feeds had the key length problem recently as well, #1044882: table feeds_item could not be created pdo exception key was too long)

In the below, it looks to me like the second one was probably created with the UI.


select name, length(name) as fieldlength from rules_config order by fieldlength desc limit 10

Yeah, I think the tendency has been toward longer names since, at least as I understand it, Drupal Commerce did not want to repeat the ubercart methodology and prefix every thing as dc/uc, and instead opted for "commerce_". It would be good if we didn't have to break that methodology for rules, and I THINK we can live in 64 (I just changed things back to 255 via the above patch any time we're working with dev, so I've not actually tested it).

commerce_checkout_order_status_update is the specific ruleset that kills most installs, but we only know that because we hacked at our dependencies until we figured it out. Hopefully we can live in 64 as well. It'd be good to hear from Ryan on this, but I think he's still on vacation.

Eclipse

ok, so here is patch fixing it to 64. Please test!

>In the below, it looks to me like the second one was probably created with the UI.
hm, I guess that might have been caused by bug that was in the machine-name generation of core. It didn't respect the max-length value, so longer values might have been auto-generated due to that. The core bug has been already fixed though, so it should not happen any more.

thanks.

ok, I've fixed that and tested it on another site. However, the rules-scheduler update failed to create the new keys via db_change_field() - I don't know why as it works for the rules update. Reverting to a db_add_unique_key() worked for me.

So this could need another tester who makes sure the upgrade works and all indexes have been properly created.

I applied #14 and the database update ran successfully.

mysql> desc rules_config;
+--------+--------------+------+-----+-----------+----------------+
| Field  | Type         | Null | Key | Default   | Extra          |
+--------+--------------+------+-----+-----------+----------------+
| id     | int(11)      | NO   | PRI | NULL      | auto_increment |
| name   | varchar(64)  | NO   | UNI | NULL      |                |
| label  | varchar(255) | NO   |     | unlabeled |                |
| plugin | varchar(127) | NO   | MUL | NULL      |                |
| active | int(11)      | NO   |     | 1         |                |
| weight | tinyint(4)   | NO   |     | 0         |                |
| status | tinyint(4)   | NO   |     | 1         |                |
| module | varchar(255) | YES  |     | NULL      |                |
| data   | longblob     | YES  |     | NULL      |                |
+--------+--------------+------+-----+-----------+----------------+
9 rows in set (0.20 sec)

thanks, committed.

I have a problem... 64 characters is too small for my name fields for Commerce Pickup module my rule name ends up looking like this: commerce_shipping_service_commerce_pickup__commerce_customer_profile__23197

I did a custom patch for my situation, is it really necessary to limit this field so much?