variable.name only 48 characters

there is no problem in increasing this limit manually. I agree it is somwhat arbitrary. Maybe it is not too late to change this in D5? If too late, please move to D6.

This is not critical

This patch increases the column size from 48 characters to 128.

I re-tested the patch against HEAD and it works fine, so I think either your patch program is the issue or you're not applying the patch in the core directory. Perhaps someone else can verify that the patch applies cleanly.

Also just for future reference, no need to change the version setting for this issue - 5.x-dev is fine (and arguably preferable).

minor tweak to make it system_update_1022, but works as advertised and it good. Watch out for update numbers if more get in first!

Re-rolled from the root directory.

Here's a version that creates system_update_2000() instead.

Re-rolled.

Oops, there was an extra space.

Just a note that this does cause issues for contrib modules, so the sooner it's fixed the better. If it's a bug (and I believe it is), could this be backported to 5.x?

Looks good to me. +1

For 6.x changes, the update needs to start with 300x

Nope, see for example http://drupal.org/node/115667

Oops! My mistake! I read that as "updates from 5.x.x to 6.x.x"

Committed to CVS HEAD. Thanks.

drumm, do you think we can convince you this is worth backporting? The current field length breaks some modules (see the link in my last comment), it's not an API change, and a longer field shouldn't be able to break any existing code.

I lean against database upgrades during a stable release for anything but soul-crushing bugs, personally. But it's Neil's call. :)

given it requires a schema change, i'm guessing the answer will be 'no', but i'll leave that for drumm or dries to say. ;)

meanwhile, i just finished going through all the CVS activity in the HEAD of core while i was gone, and documenting everything that seemed developer-visible for the upgrade docs. this issue was one of them, so i added a (very short) blurb here:

http://drupal.org/node/114774#variable-name-size

so, once someone sets this back to 6.x/fixed, they can rest assured that it's already been added to the upgrade docs. ;)

cheers,
-derek

This patch as committed removes the primary key from the variable table on pgsql because db_change_column() drops it and the patch does not explicitly re-add it (the problem was revealed by schema.module's comparison report). This means that every call to variable_set() has to do a full table search, an unacceptable performance penalty.

I've attached a new patch that fixes the problem. It rewrites the update to use the schema api though doing so is not necessary. It would be equally valid to add db_add_primary_key($ret, 'variable', array('name')); to the pgsql block after the call to db_change_column().

I have tested this patch, but it is currently impossible to test it without hacking cache.inc to disable all caching because of upgrade problems involving the 'serialized' column. So applying this patch should probably wait until it can be tested against a non-hacked HEAD.

Looks like the last patch was still on 6.x. Please get that figured out before thinking about 5.x.

Fixed by http://drupal.org/node/157682