The variable table name field is set at varchar 48 in the database description:

--
-- Table structure for table 'variable'
--

CREATE TABLE variable (
  name varchar(48) NOT NULL default '',
  value longtext NOT NULL,
  PRIMARY KEY (name)
)

This is a problem if you're using CCK and Pathauto. I've found that I get weird errors with Pathauto because it is truncating the name as the value being passed in is greater than 48 chars. Increasing this to 64 worked for me.

I'm not sure if this should be mentioned on Pathauto or here, but it seemed like other modules could cause the same problem.

Files: 
CommentFileSizeAuthor
#3 longername.patch607 bytesRobRoy

Comments

darius’s picture

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

Darius

RobRoy’s picture

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.

RobRoy’s picture

Status: Active » Needs review
FileSize
607 bytes

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

pfaocle’s picture

Status: Needs review » Needs work

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.

killes@www.drop.org’s picture

Version: 4.7.0 » x.y.z

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

arthurf’s picture

In the interests of not having name space collisions and having greater module flexibility, doesn't it make sense to have a larger field? What's the penalty to making the field bigger?

webchick’s picture

Category: bug » feature

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.

arthurf’s picture

Ahhh. Makes total sense. Thanks for the answer! Do I need to do anything to lobby to have it included for 4.8?

tatere’s picture

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.

xpereta’s picture

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.

arthurf’s picture

So it sounds like there are two changes that need to happen:

1) return an error when the variable name is to long
2) increase the size of the variable name field

greggles’s picture

Version: x.y.z » 6.x-dev

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.

hendler’s picture

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

webchick’s picture

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?

Owen Barton’s picture

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

RobRoy’s picture

Title: Variable table needs larger name field » Throw watchdog error if variable name maxlength is exceeded

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.

dmitrig01’s picture

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
}
Tyrael’s picture

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

dpearcefl’s picture

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

Owen Barton’s picture

Title: Throw watchdog error if variable name maxlength is exceeded » Handle long configuration key names gracefully
Version: 6.x-dev » 8.x-dev
Status: Needs work » Active
Issue tags: +Configuration system

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.

heyrocker’s picture

Status: Active » Fixed

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

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sun’s picture