Closed (fixed)
Project:
Drupal core
Version:
6.x-dev
Component:
node system
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Anonymous (not verified)
Created:
9 Mar 2007 at 13:11 UTC
Updated:
29 Dec 2022 at 23:28 UTC
Jump to comment: Most recent file
Comments
Comment #1
ChrisKennedy commentedHere's a quick patch to increase it from 128 to 255. I haven't had time to test it unfortunately.
Comment #2
ChrisKennedy commentedAssigning.
Comment #3
ChrisKennedy commentedSynced and tested to work fine on MySQL at least.
Comment #4
ChrisKennedy commentedSynced.
Comment #5
keith.smith commented:( patch no longer applies cleanly
# patch -p0 < node_title_255_1.patch
patching file modules/node/node.module
Hunk #1 succeeded at 3148 (offset 98 lines).
patching file modules/system/system.install
Hunk #1 FAILED at 380.
Hunk #2 FAILED at 417.
Hunk #3 FAILED at 866.
Hunk #4 FAILED at 903.
Hunk #5 succeeded at 3362 (offset -503 lines).
4 out of 5 hunks FAILED -- saving rejects to file modules/system/system.install.rej
Comment #6
asimmonds commentedUpdated patch to HEAD and schema API
Comment #7
webchickTested (including upgrade path) and works. RTBC.
Comment #8
hickory commentedPlease make the title field TEXT rather than just imposing another arbitrary limit (which still isn't long enough). Patch here: http://drupal.org/node/103083
Comment #9
webchickThis is still a patch, and still RTBC. Whether we should keep it as varchar or migrate to text would be up to the core committers, however I have the sneaking suspicion that a text field might have performance implications...
Comment #10
dries commentedCommitted to CVS HEAD. Thanks.
Comment #11
yched commentedI don't think the upgrade path is reliable. You can't rely on the 'current' state of a schema in an upgrade function, because you can't predict what the schema will be by the time the update is executed (I should _really_ bookmark this link where Barry explains that much better than I do). Probably not that serious in this particular case, but it sets bad example anyway.
Attached patch corrects the upgrade path to what I understand to be the right way.
Side note : that's an easy mistake to make, and db_update_field function sure is tempting - should we really leave it as an API function or prefix it with '_' ?
Comment #12
ChrisKennedy commentedYeah it is a little confusing. I think it should just be pointed out in the documentation for db_update_field.
In any case, the patch looks good and works fine.
Comment #13
asimmonds commentedSorry, that was my mistake. I had looked at system_update_6019() as a reference but probably should have done a bit more research on schema updates.
Comment #14
yched commentedtrue, system_update_6019 looks funny. Then again, I think it's precisely one of the schema patch updates (probably written by barry ?) so I'm not sure what to think about those. A word of comment could probably be used here ?
Comment #15
bjaspan commentedyched is right. db_update_field() is not safe for hook_update_n() functions, system_update_6019() is broken, and his patch in #11 is the correct solution.
This situation is not only about the node title field so I created a new issue for it: http://drupal.org/node/156741. I think this issue should be changed back to 'fixed' as node titles can now be 255 chars as requested.
Comment #16
dries commentedThanks for chiming in bjaspan. Committed.
Comment #17
yched commentedBarry, in http://drupal.org/node/156741 you write :
db_change_field() does not automatically re-create any keys or indexes in which changed column is involved, so if the column is involved in a key or index it must be explicitly dropped and re-created.
node.title is used in the following index (node.schema) :
'node_title_type' => array('title', array('type', 4))
So i guess this means we are missing the index too here ?
Comment #18
bjaspan commentedHuh. Yes. The funny thing is I specifically checked for that before saying this should be marked fixed, but somehow I missed it.
So, I'm sorry for saying the patch was correct. It needs a db_drop_index() and db_add_index() wrapped around the call to db_change_field() for node.title. I am pretty confident that after making this update on pgsql, schema.module will report that the node table is missing an index. This doesn't happen on mysql due to the different way column changes are handled. I am unable to test this at the moment because head update is too broken (in part because of http://drupal.org/node/152383 killing INSERT INTO {batch}).
I am currently immersed in trying to get update.php working on pgsql well enough to test http://drupal.org/node/156741. When I'm done, I'll include this fix in that patch since it is basically for the same problem (there will be several required index drop/adds for that patch, too).
Comment #19
bjaspan commentedI have verified that, on pgsql, running system_update_6025 causes schema.module to report: "node: indexes node_title_type: missing in database".
Comment #20
(not verified) commented