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.

CommentFileSizeAuthor
#4 database_forum.patch1.27 KBSouvent22
#2 ref_error_0.patch4.64 KBSouvent22
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

syllance’s picture

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.

Souvent22’s picture

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

Souvent22’s picture

Status: Active » Needs review

Update status; anyone wanna take a look?

Souvent22’s picture

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

Souvent22’s picture

Assigned: Unassigned » 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.

syllance’s picture

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

robertDouglass’s picture

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.

syllance’s picture

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 ?

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good to me.

Dries’s picture

Status: Reviewed & tested by the community » Needs work

There is no patch?

robertDouglass’s picture

robertDouglass’s picture

m3avrck’s picture

Status: Needs work » Reviewed & tested by the community

Yes patch is in comment #4 and as I said above, code looks correct to me.

Cvbge’s picture

Looks good for me too.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)