[drupal-devel] [bug] Forum table: vid gone AWOL

[robertDouglass]

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:   robertDouglass
 Status:       patch (code needs work)

yes there is:
http://drupal.org/files/issues/ref_error_0.patch




robertDouglass



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?