Handle long configuration key names gracefully

This is causing problems with relativity.module, too. The variable name field should be longer. Why 48, after all?

Darius

Just ran into this. This needs to be 128 at least right? Or should it be maxed out at 255?

Using CCK and per-type settings like 'menu_panel_visibility_defaults_content-before__after' I get errors cause it's too long.

I'm not sure about pgsql, but what about this for a start?

Couple of things:

• As you mention, PGSQL needs taken care of
• (I think) the main .mysql and .pgsql files in /database will also need updating

The change seems fair enough, though - probably needs some more feedback from people with some in-depth knowledge of the Drupal database and any adverse impact this may have.

I don't consider this as cneeded for 4.7, module authors should use shorter variable names.

arthurf: The big downside is a database change in a stable release that is to have no database changes between versions unless absolutely necessary (fixing critical bugs, etc.).

Sounds like a good idea for 4.8/5.0 though.

variable_set() needs to know the maximuim field name length and issue an error or at least a warning when it's exceeded. otherwise this can be a silent failure. this is especially true if you are not going to change the field length in the database table for quite some time, since that's the easiest temporary fix.

Maybe this behaviour is by design. The variable_set always assumes that the operation is successful, it seems that therefore is the responsability of the users of the function to always check that the variable was updated successfuly, for example fetching the variable again and check its value.

As tatere suggests, wouldn't make sense that variable_set checked the size of the $name parameter and raised an error when appropiate?

However the problems I have encountered seem to happen when modules use (abuse?) the variable mechanism to create variable names that depend on input entered by the user. In this case the module should check that the input of the user does not prevent the variables to be stored.

It makes sense to me to do both of the things listed in Arthurf's comment #11

I believe there is some concern about larger columns affecting performance since this table is central to basically every drupal operation. It would be good to benchmark any changes.

I agree with arthurs comments on #11.

I have written some custom code in the past to deal with this - and made it conditional modification depending on MySQL 4 or 5 - where MySQL 5 has 512 for varchars (probably not needed here).

I'm not sure about an error message...

a) we need to assume that most users are not developers.
b) good error messages need to state steps to resolve the problem. "Go into your database and make the variable.name field bigger" is not an acceptable fix for most people. ;P

Is there any reason not to just jack the field up all the way to 255 chars and be done with it? The contents of this table are only retrieved once and then cached, no?

+1 for both increasing the field length and also adding a length check in variable_set()

Some benchmarking would be nice, although I can't see how this is likely to cause any noticeable difference.

This already got bumped up to 128 in system_update_2002() in http://drupal.org/node/103634 just in case you didn't know. So I'm changing the title to reflect that we need some sort of check if we exceed that. If we want to bump it up to 255 maybe reopen that issue or change this title? I think 255 would be plenty and would probably avoid us having to worry about warnings.

why not md5 the variable name, and have a hook_variables, where modules can put the data in their own tables. So, if I had 'realvarnames.module', it would look like

function realvarnames_variables($name, $md5, $action)
{
  db_query("INSERT INTO realvarnames (name, md5
}

md5 hash isn't a unique key.

it's possible, to 2 different key have the same md5 hash.
it's very rare case, but not impossible.

Tyrael

Given the the variable name is now 128 characters, do we still want to have a check for the length? Or drop this issue?

Given the lack of reports here since 2007, I think it is fairly safe to assume that the 128 limit is sufficient for the vast majority of cases (of course it is possible that there were issues, but no one found this!). For D6 & D7 I guess devel or some other module could perhaps add a check and display a warning if variables exist that are at the limit (suggesting that it may have been exceeded).

For D8, I think the new system being built for the configuration management initiative should be able to handle this - it seems the best bet is to leave this open as a reminder and close once we have confirmed the new code can handle long key names gracefully.

The new config system has 255 character naming, so this will not be an issue. Marking fixed.