[drupal-devel] forum table WTF
Hi, Looking at the code and database table of the forum module, I don't understand the role of the forum table to be honest. The forum table is now only a nid->tid relation, while the term_node table would suffice to express such relations. Term_node does not limit a node to only have one parent term, but that is the sole difference it seems. Some queries in forum module use both the forum table and the term_node table, some only the latter. Goba
As far as I see, it is hardly similar, since I am talking about nid->tid relationships, and that bug is about the top level (the vocabulary itself), and that bug has nothing to do with where nid->tid relations are stored. Goba
Goba, have you seen this similar bug? http://drupal.org/node/24274
I am buildng the extended forum module (a trip_forum.module descendant) on top of forum.module and found "forum" table quite a nuisance, since I need to use multi-vocabulry relations for each forum node. It's ok to streamline forum.module configuration UI to be single-parent, single-vocabulary, but in the background, low level, it should allow all kind of taxonomy freedom
Gabor Hojtsy wrote:
Hi,
Looking at the code and database table of the forum module, I don't understand the role of the forum table to be honest. The forum table is now only a nid->tid relation, while the term_node table would suffice to express such relations. Term_node does not limit a node to only have one parent term, but that is the sole difference it seems. Some queries in forum module use both the forum table and the term_node table, some only the latter.
Goba
yeah, this looks redundant to me too.
On 06 Jun 2005, at 10:13, Gabor Hojtsy wrote:
Looking at the code and database table of the forum module, I don't understand the role of the forum table to be honest. The forum table is now only a nid->tid relation, while the term_node table would suffice to express such relations. Term_node does not limit a node to only have one parent term, but that is the sole difference it seems. Some queries in forum module use both the forum table and the term_node table, some only the latter.
The table might be used for the 'Leave shadow copy' feature ... (random guess)? If we can improve the performance of the forum module, by all means. It would be a tremendous improvement for sites like drupal.org, spreadfirefox.com, etc. I don't mind removing the 'Leave shadow copy' feature if it makes the code simpler, improves performance and de-clutters the node edit form. Are you going to investigate this some more? -- Dries Buytaert :: http://www.buytaert.net/
Looking at the code and database table of the forum module, I don't understand the role of the forum table to be honest. The forum table is now only a nid->tid relation, while the term_node table would suffice to express such relations. Term_node does not limit a node to only have one parent term, but that is the sole difference it seems. Some queries in forum module use both the forum table and the term_node table, some only the latter.
The table might be used for the 'Leave shadow copy' feature ... (random guess)? If we can improve the performance of the forum module, by all means. It would be a tremendous improvement for sites like drupal.org, spreadfirefox.com, etc.
I don't mind removing the 'Leave shadow copy' feature if it makes the code simpler, improves performance and de-clutters the node edit form.
Are you going to investigate this some more?
Here are the queries where {forum} is used: // forum_term (when a term is deleted, the results // are run through node_delete, so forum nodes are deleted) 'SELECT f.nid FROM {forum} f WHERE f.tid = %d', $object->tid // from forum_load 'SELECT * FROM {forum} WHERE nid = %d', $node->nid // forum_update 'UPDATE {forum} SET tid = %d WHERE nid = %d', $node->tid, $node->nid // forum_insert 'INSERT INTO {forum} (nid, tid) VALUES (%d, %d)', $node->nid, $node->tid // forum_delete 'DELETE FROM {forum} WHERE nid = %d', $node->nid // forum_get_topics: "SELECT n.nid, f.tid, n.title, n.sticky, u.name, u.uid, n.created AS timestamp, n.comment AS comment_mode, l.last_comment_timestamp, IF(l.last_comment_uid, cu.name, l.last_comment_name) AS last_comment_name, l.last_comment_uid, l.comment_count AS num_comments FROM {node} n, {node_comment_statistics} l, {users} cu, {term_node} r, {users} u, {forum} f WHERE n.status = 1 AND l.last_comment_uid = cu.uid AND n.nid = l.nid AND n.nid = r.nid AND r.tid = %d AND n.uid = u.uid AND n.nid = f.nid" It seems that the nid->tid relationship is maintained, loaded, inserted, updated, removed, but then only used in the forum_get_topics query and only to ensure (on top of term_node) that the nids match. If this is connected to the shadow copy feature, then this query is supposed to select the 'primary term' for this particular node, and this is why the whole maintanance stuff is needed. From the implementation of the shadow copy feature, it stores all the relations in the term_node table, and maintains no information on what is the primary term for a forum node. Otherwise I see no reason to keep this table. Goba
On Mon, 6 Jun 2005, Gabor Hojtsy wrote:
It seems that the nid->tid relationship is maintained, loaded, inserted, updated, removed, but then only used in the forum_get_topics query and only to ensure (on top of term_node) that the nids match. If this is connected to the shadow copy feature, then this query is supposed to select the 'primary term' for this particular node, and this is why the whole maintanance stuff is needed. From the implementation of the shadow copy feature, it stores all the relations in the term_node table, and maintains no information on what is the primary term for a forum node.
IIRC, the "shadow copy" feature was simplified to not have a "primary tid" anymore about a year ago.
Otherwise I see no reason to keep this table.
I agree. Will also remove the need to do extra handling of forum nodes in the upgrade path of my revisions patch. Cheers, Gerhard
It seems that the nid->tid relationship is maintained, loaded, inserted, updated, removed, but then only used in the forum_get_topics query and only to ensure (on top of term_node) that the nids match. If this is connected to the shadow copy feature, then this query is supposed to select the 'primary term' for this particular node, and this is why the whole maintanance stuff is needed. From the implementation of the shadow copy feature, it stores all the relations in the term_node table, and maintains no information on what is the primary term for a forum node.
IIRC, the "shadow copy" feature was simplified to not have a "primary tid" anymore about a year ago.
Well, it does not seem so. The query I posted from forum_get_topics() is from latest Drupal CVS (forum module 1.256), and it only selects topics (nodes) registered in the forum table. And since the forum table has a nid primary key.. TADA, we found the "primary tid". Here is the important part: SELECT n.nid, f.tid ... FROM {node} n, ... {forum} f WHERE n.nid = f.nid Goba
On Mon, 6 Jun 2005, Gabor Hojtsy wrote:
IIRC, the "shadow copy" feature was simplified to not have a "primary tid" anymore about a year ago.
Well, it does not seem so. The query I posted from forum_get_topics() is from latest Drupal CVS (forum module 1.256), and it only selects topics (nodes) registered in the forum table. And since the forum table has a nid primary key.. TADA, we found the "primary tid".
Here is the important part:
SELECT n.nid, f.tid ... FROM {node} n, ... {forum} f WHERE n.nid = f.nid
I agree, but the shadowed tid isn't rendered special in any way. Cheers, Gerhard
IIRC, the "shadow copy" feature was simplified to not have a "primary tid" anymore about a year ago.
Well, it does not seem so. The query I posted from forum_get_topics() is from latest Drupal CVS (forum module 1.256), and it only selects topics (nodes) registered in the forum table. And since the forum table has a nid primary key.. TADA, we found the "primary tid".
Here is the important part:
SELECT n.nid, f.tid ... FROM {node} n, ... {forum} f WHERE n.nid = f.nid
I agree, but the shadowed tid isn't rendered special in any way.
Sure, rendering is a different question, this is why this stuff had an easy time hiding. Goba
participants (6)
-
Dries Buytaert -
Gabor Hojtsy -
Gerhard Killesreiter -
Kristjan Jansen -
Moshe Weitzman -
Robert Douglass