Issue status update for http://drupal.org/node/31343 Post a follow up: http://drupal.org/project/comments/add/31343 Project: Drupal Version: cvs Component: forum.module Category: bug reports Priority: critical Assigned to: Souvent22 Reported by: robertDouglass Updated by: Cvbge Status: patch (ready to be committed) Looks good for me too. Cvbge Previous comments: ------------------------------------------------------------------------ Thu, 15 Sep 2005 21:02:54 +0000 : robertDouglass The function forum_load consists entirely of this query: function forum_load($node) { $forum = db_fetch_object(db_query('SELECT * FROM {forum} WHERE vid = %d', $node->vid)); return $forum; } The forum table, however, has no vid column. What should it be? WHERE nid = %d ? I haven't been following the changes to forum.module closely, so I don't know the rest of the issues involved, but the module doesn't work as is. ------------------------------------------------------------------------ Fri, 16 Sep 2005 17:20:49 +0000 : syllance There's also en error when simply adding forum topic : user error: Unknown column 'vid' in 'field list' query: INSERT INTO forum (nid, vid, tid) VALUES (6, 6, 2) in /drupal/head/database.mysql.inc on line 99. Sounds like the removed vid field in forum table is still used in forum.module queries. The forum topic node is inserted, but not linked with forum due to this error, so that's blocking the use of forums. I've quickly searched but could not find any patch for this. I'll double check and try to make one if needed. ------------------------------------------------------------------------ Fri, 16 Sep 2005 17:55:55 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/ref_error_0.patch (4.64 KB) Yes, vid should be the second column, just as the query says, INSERT INTO....(nid, vid,tid).... Seems you have the code for this, but you havne't updated your database with update.inc, i'm not sure if there is an update to the forum table in update.inc..... Well, nevermind, a quick check shows that in updates.inc, it does alter your tables for you and fix the problem, however, the current schema in database.mysql and database.pgsql does *not*. Here's a patch that fixes that. So in short, form.module has been updated to use revisions, however, you're missing a vid field in your table. ------------------------------------------------------------------------ Fri, 16 Sep 2005 17:58:04 +0000 : Souvent22 Update status; anyone wanna take a look? ------------------------------------------------------------------------ Fri, 16 Sep 2005 18:01:31 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/database_forum.patch (1.27 KB) Crap, wrong patch. :). Too many things at once, sheesh. Get stable already Drupal! :). Anyway, here's the right one....I aplogize, ignore the other ones. ------------------------------------------------------------------------ Fri, 16 Sep 2005 18:06:57 +0000 : Souvent22 Just to clairfy, update_146() in updates.inc does correctly update the forum table and add the vid column. However, the databaswe creation scripts (database.mysql, database.pgsql) do NOT have the correct new schema for the forum table. ------------------------------------------------------------------------ Sat, 17 Sep 2005 01:14:28 +0000 : syllance I was surprised to find such an issue, even in head :) And i was right. I've been reading the node revisions changes, and just thought the database script were not updated yet. I simply did not thought to check the update script. That will be in my check list now, as everything works fine after running the update. Thanks for your quick answer, and thanks again to everyone making drupal such a nice thing :) ------------------------------------------------------------------------ Sat, 17 Sep 2005 10:11:26 +0000 : robertDouglass If all that is missing is the vid column from the database creation scripts, this patch should be a no brainer. Here is the pertinent SQL patch for those on the list that don't want to look for the patch: CREATE TABLE forum ( nid integer NOT NULL default '0', + vid integer NOT NULL default '0', tid integer NOT NULL default '0', shadow integer NOT NULL default '0', PRIMARY KEY (nid) ); CREATE INDEX forum_tid_idx ON forum(tid); +CREATE INDEX forum_vid_idx ON forum(vid); CREATE TABLE forum ( nid int(10) unsigned NOT NULL default '0', + vid int(10) unsigned NOT NULL default '0', tid int(10) unsigned NOT NULL default '0', PRIMARY KEY (nid), + KEY vid (vid), KEY tid (tid) ) TYPE=MyISAM; It looks good to me. ------------------------------------------------------------------------ Sat, 17 Sep 2005 11:58:00 +0000 : syllance i've quickly checked on my test sandbox, and there's a "duplicate" entry error when creating a forum topic revision. the update is to add the vid field, create a primary key on it, and make nid a non-primary key. i'll try to provide updated patches later today, but mysql changes should something like : CREATE TABLE forum ( nid int(10) unsigned NOT NULL default '0', + vid int(10) unsigned NOT NULL default '0', tid int(10) unsigned NOT NULL default '0', - PRIMARY KEY (nid), + KEY (nid), + PRIMARY KEY vid (vid), KEY tid (tid) ) TYPE=MyISAM; (manual patch :p, i'll do the thing later) while talking database, do you know if there's any specific reason why all drupal tables are forced MyISAM type ? ------------------------------------------------------------------------ Sat, 17 Sep 2005 16:27:24 +0000 : m3avrck Code looks good to me. ------------------------------------------------------------------------ Sun, 18 Sep 2005 11:27:33 +0000 : Dries There is no patch? ------------------------------------------------------------------------ Sun, 18 Sep 2005 11:36:40 +0000 : robertDouglass yes there is: http://drupal.org/files/issues/ref_error_0.patch ------------------------------------------------------------------------ Sun, 18 Sep 2005 11:37:48 +0000 : robertDouglass Oops: http://drupal.org/files/issues/database_forum.patch ------------------------------------------------------------------------ Sun, 18 Sep 2005 13:33:44 +0000 : m3avrck Yes patch is in comment #4 and as I said above, code looks correct to me.