On a freshly upgraded 4.7-beta4 site (from 4.7-beta2):

When viewing a forum page anonymously, the following error is shown:

user warning: Unknown column 'n.nid' in 'on clause' query: SELECT DISTINCT(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 != 0, 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 INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all'))) AND n.status = 1 AND l.last_comment_uid = cu.uid AND n.nid = l.nid AND n.nid = r.nid AND r.tid = 1 AND n.uid = u.uid AND n.vid = f.vid ORDER BY n.sticky DESC, l.last_comment_timestamp DESC, n.created DESC LIMIT 0, 50 in /home/www/drupal-4.7.0-beta4/includes/database.mysql.inc on line 124.

No forum posts are shown in the listing.

If I log in, the error goes away, and the forum index is shown.

Flagged issue as forum.module, but I'm really not sure that's it.

CommentFileSizeAuthor
#9 forum.module_10.patch1.4 KBstmind
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stmind’s picture

Note that this error does not occur on a fresh install.

chx’s picture

I vividly remember JonBob introducing node_access_view_all_nodes and moshe, JonBob and I patching a bug related to it in Brussels at the first DrupalCon http://drupal.org/node/18123. And now I am heading to Vancouver to meet these great guys again! I love February!

Erm, where was I? Yes. WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all'))) this should not be in the query (because of said function). If it is, then you have (or had?) some node access module which is causing problem.

stmind’s picture

I tracked it down to something in taxonomy_access (don't know what more than that yet). I set up a new site and added a forum and taxonomy_access, and as soon as I set up a rule for category access the error appeared.

stmind’s picture

Project: Drupal core » Taxonomy Access Control
Version: 4.7.0-beta4 » 4.6.x-1.x-dev
Component: forum.module » Code

Moving to taxonomy_access project.

stmind’s picture

Version: 4.6.x-1.x-dev » 4.7.x-1.x-dev
Priority: Normal » Critical
stmind’s picture

Below is an updated version of the error. I think the first error was taken with taxonomy_access disabled. This one was with the fresh site, just after enabling taxonomy_access:

user warning: Unknown column 'n.nid' in 'on clause' query: SELECT DISTINCT(n.nid), l.last_comment_timestamp, IF(l.last_comment_uid != 0, cu.name, l.last_comment_name) as last_comment_name, l.last_comment_uid FROM node n, node_comment_statistics l, users cu, term_node r INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 1 AND na.realm = 'term_access'))) AND n.nid = r.nid AND r.tid = 1 AND n.status = 1 AND n.type = 'forum' AND l.last_comment_uid = cu.uid AND n.nid = l.nid ORDER BY l.last_comment_timestamp DESC LIMIT 0, 1 in /home/www/drupal-4.7.0-beta4/includes/database.mysql.inc on line 124.

stmind’s picture

When I disable taxonomy_access under it's settings menu, it resets the node_access table and the error goes away. node_access looks like this:

mysql> select * from node_access;
+-----+-----+-------+------------+--------------+--------------+
| nid | gid | realm | grant_view | grant_update | grant_delete |
+-----+-----+-------+------------+--------------+--------------+
|   0 |   0 | all   |          1 |            0 |            0 |
+-----+-----+-------+------------+--------------+--------------+
1 row in set (0.00 sec)

When I enable the module again (under it's settings menu), node_access looks like this, and the error returns:

mysql> select * from node_access;
+-----+-----+-------------+------------+--------------+--------------+
| nid | gid | realm       | grant_view | grant_update | grant_delete |
+-----+-----+-------------+------------+--------------+--------------+
|   1 |   1 | term_access |          1 |            0 |            0 |
|   1 |   2 | term_access |          0 |            0 |            0 |
|   2 |   1 | term_access |          0 |            0 |            0 |
|   2 |   2 | term_access |          1 |            0 |            0 |
+-----+-----+-------------+------------+--------------+--------------+
4 rows in set (0.00 sec)
stmind’s picture

Project: Taxonomy Access Control » Drupal core
Version: 4.7.x-1.x-dev » 4.7.0-beta4
Component: Code » forum.module

Ok....

So this SQL statement fails:

SELECT 
	DISTINCT(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 != 0, 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
INNER JOIN 
	node_access na ON n.nid = na.nid 
WHERE 
	(na.grant_view >= 1 AND	((na.gid = 0 AND na.realm = 'all') OR (na.gid = 1 AND na.realm = 'term_access'))) AND
	n.status = 1 AND
	l.last_comment_uid = cu.uid AND
	n.nid = l.nid AND
	n.nid = r.nid AND
	r.tid = 1 AND 
	n.uid = u.uid AND 
	n.vid = f.vid

ORDER BY 
	n.sticky DESC,
	l.last_comment_timestamp DESC,
	n.created DESC
LIMIT 0, 50

This is just an error in the FROM clause, the join is in the wrong place. If we change the order of the FROM, and place the join after "node n", where it should be, the statement works:

SELECT 
	DISTINCT(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 != 0, cu.name, l.last_comment_name) AS last_comment_name,
	l.last_comment_uid, 
	l.comment_count AS num_comments 
FROM 
	node n  INNER JOIN node_access na ON n.nid = na.nid,
	node_comment_statistics l, 
	users cu, 
	term_node r, 
	users u, 
	forum f
WHERE 
	(na.grant_view >= 1 AND	((na.gid = 0 AND na.realm = 'all') OR (na.gid = 1 AND na.realm = 'term_access'))) AND
	n.status = 1 AND
	l.last_comment_uid = cu.uid AND
	n.nid = l.nid AND
	n.nid = r.nid AND
	r.tid = 1 AND 
	n.uid = u.uid AND 
	n.vid = f.vid

ORDER BY 
	n.sticky DESC,
	l.last_comment_timestamp DESC,
	n.created DESC
LIMIT 0, 50

So I've done a little more hunting, and I've come across this statement in forum.module, function forum_get_topics(), line 740:

$sql = db_rewrite_sql("SELECT n.nid, n.nid as TESTING, 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 != 0, 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.vid = f.vid");

This is the statement that's being rewritten, with the join added, and breaking. If I change the line to look like this (simply move the "node n" to the last element of the FROM clause):

$sql = db_rewrite_sql("SELECT n.nid, n.nid as TESTING, 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 != 0, cu.name, l.last_comment_name) AS last_comment_name, l.last_comment_uid, l.comment_count AS num_comments FROM {node_comment_statistics} l, {users} cu, {term_node} r, {users} u, {forum} f, {node} n 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.vid = f.vid");

.... it appears to fix the error, and the forum list is shown properly.

So, I'm moving this issue back to the Drupal project, and I will attach this as a patch momentarily.

I'm not confident this is the proper way to handle it, so I urge someone more knowlegable on the db_rewrite_sql() function to weigh in and review this.

Thanks.

stmind’s picture

Status: Active » Needs review
FileSize
1.4 KB

Patch attached

killes@www.drop.org’s picture

Priority: Critical » Normal
Status: Needs review » Reviewed & tested by the community

Excellent investigation. The reason why it fails is that the regexp somehow expects the explicit form of a join (FROM foo INNER JOIN bar ...) to be used. However, the query is already long and unreadable enough, so we don't want to change that.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

rjl’s picture

Status: Fixed » Needs review

I don't know if you are allowed to re-open an issue marked as fixed but I have a question about this issue and would like there to be a little further review. If you would like me to post this in a different place, please tell me where, this just seemed the most appropriate place to do it. Again, sorry if I am in the wrong.

killes noted...

Excellent investigation. The reason why it fails is that the regexp somehow expects the explicit form of a join (FROM foo INNER JOIN bar ...) to be used. However, the query is already long and unreadable enough, so we don't want to change that.

I am unclear which regexp he is referring to in his comment, but I think it fails because of the db_rewirte_query function in the database.inc file, here is the code (this is very possibly the regexp killes was referring to)

if (!empty($join)) {
  $query = preg_replace('/LEFT |RIGHT |INNER |WHERE|GROUP|ORDER|$/', $join .' \0', $query, 1);
}

This expression looks for an SQL clause predicate that would assumably come after the FROM clause predicate. It inserts the value of $join (any module implementing hook_db_rewrite_sql() can add a JOIN clause to $join). The node module implements this hook which is how the INNER JOIN node_access na ON n.nid = na.nid found its way into the SQL statement referenced above.

I should note that with the db_rewrite_sql() function, there are some additional parameters, here is the notes section from the file

/**
 * Rewrites node, taxonomy and comment queries. Use it for listing queries.
 *
 * @param $query
 *   Query to be rewritten.
 * @param $primary_table
 *   Name or alias of the table which has the primary key field for this query. Possible values are: comments, forum, node, term_data, vocabulary.
 * @param $primary_field
 *   Name of the primary field.
 * @param $args
 *   An array of arguments, passed to the implementations of hook_db_rewrite_sql.
 * @return
 *   The original query with JOIN and WHERE statements inserted from hook_db_rewrite_sql implementations. nid is rewritten if needed.
 */

The query above called the function without expressing these values, allowing the function to use it's default values of $primary_table='n' and $primary_field='nid'

Because this SQL statements FROM clause contains no JOIN/ON clauses the joins are assumed to be cross joins. So, in this case, the fix recommended here was to move the node table (or better said the $primary_table) to the last position in the FROM clause. And it works in this situation.

But...

What about a module like taxonomy? It calls db_rewrite_sql() in 15 different places. In 5 of them, the FROM clause contains mutliple tables with no join clauses as follows

FROM {term_data} t, {term_node} r 
FROM {term_hierarchy} h, {term_data} t
FROM {term_hierarchy} h, {term_data} t 
FROM {term_hierarchy} h, {term_data} t 
FROM {term_data} t, {term_hierarchy} h 

All 5 statements use $primary_table='t' and $primary_field='tid'
So if we are to follow the logic about moving the table indicated as the $primary_table to the last position in the FROM clause, then in the 5 taxonomy examples, the first and last items would also have to be changed, Yes?

So I don't think the recommended fix is necessarily the right solution, unless a little more is understood about the db_rewrite_sql function and hook. There have been some similar problems with node access modules interacting with other modules such as event node/43735 and image node/40623. (note both of these reference mysql 5.0).

I think the problems are more about the db_rewrite_sql function, which is a great function, but it should be clearer on issues related to the value of $query especially in terms of the FROM and whether or not it has multiple tables and if so are the joins expressed as clauses or implied cross joins. Because, if you send a typical FROM clause with multiple tables using JOIN clauses then the value of $join is inserted after the first table and before the first JOIN clause (assuming I am correct).

Is any of this making sense to anyone?

Not really sure about any of this, except that review and clarification are needed.

------- Related - maybe should be separate post ?? ------

This sort of leads to a related issue on use of JOINS in FROM clauses
Lets take a look at taxonomy module, here's an example found in the taxonomy_term_count_nodes() function

  if (!isset($count[$type])) {
    // $type == 0 always evaluates true is $type is a string
    if (is_numeric($type)) {
      $result = db_query(db_rewrite_sql('SELECT t.tid, COUNT(n.nid) AS c FROM {term_node} t INNER JOIN {node} n ON t.nid = n.nid WHERE n.status = 1 GROUP BY t.tid'));
    }
    else {
      $result = db_query(db_rewrite_sql("SELECT t.tid, COUNT(n.nid) AS c FROM {term_node} t, {node} n WHERE t.nid = n.nid AND n.status = 1 AND n.type = '%s' GROUP BY t.tid"), $type);
    }
    while ($term = db_fetch_object($result)) {
      $count[$type][$term->tid] = $term->c;
    }
  }

The 2 SQL strings are basically the same query, both use the condition t.nid = n.nid, but the first one joins the tables and places the condition in the ON clause and the second one uses an implied cross join with the condition in the WHERE clause.

Why I included it here, is that, if the logic above follows, the first query should present errors with a node access module for a user without permissions.

There's a small section in the handbooks on naming conventions node/2497 but nothing really addressing this issue. I think most would agree that it doesn't probably matter, but consistency would be preferred. I have always been taught that placing a condition in an ON clause is faster, but I actually don't know where I learned that, so it may not be true. For me personally, a FROM clause with JOIN clauses is easier to read. I note killes comment above the query is already long and unreadable enough, so we don't want to change that, this is arguement enought for me that it should be changed. Killes is an expert, and if it's hard for him, imagine how hard it is for someone with lesser skills.

Sorry if I am totally off mark, if I am missing the point on these issues, can someone help me understand it better.

jyang’s picture

I got the same error each time when I log out from admin, then log in as another user or stay anonymously, the problem come from statistics and "popular content" set up, if I disable both in settings and blocks, the problem is gone. Once I enble it, the problem shows up again.Don't know how to fix it.

user warning: Unknown column 'n.nid' in 'on clause' query: SELECT DISTINCT(n.nid), n.title, u.uid, u.name FROM node_counter s INNER JOIN node_access na ON na.nid = n.nid INNER JOIN node n ON s.nid = n.nid INNER JOIN users u ON n.uid = u.uid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 0 AND na.realm = 'og_public') OR (na.gid = 0 AND na.realm = 'og_all'))) AND s.daycount <> '0' AND n.status = 1 ORDER BY s.daycount DESC LIMIT 0, 3 in C:\progs\web\Apache\Apache2\htdocs\Drupal\drupal-4.7.0-beta4\includes\database.mysql.inc on line 124.

Julien PHAM’s picture

Version: 4.7.0-beta4 » x.y.z
Status: Needs review » Active

I have the same errors, and I have latest CVS. Seems there are other places where the query is not correct when anonymous.

user warning: Unknown column 'n.nid' in 'on clause' query: SELECT DISTINCT(n.nid), l.last_comment_timestamp, IF(l.last_comment_uid != 0, cu.name, l.last_comment_name) as last_comment_name, l.last_comment_uid FROM node n, node_comment_statistics l, users cu, term_node r INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 1 AND na.realm = 'term_access'))) AND n.nid = r.nid AND r.tid = 16 AND n.status = 1 AND n.type = 'forum' AND l.last_comment_uid = cu.uid AND n.nid = l.nid ORDER BY l.last_comment_timestamp DESC LIMIT 0, 1 in /homepages/23/d154239500/htdocs/min/includes/database.mysql.inc on line 118.
Julien PHAM’s picture

Priority: Normal » Critical
Julien PHAM’s picture

btw this happens with authenticated users too, but not when I login as an administrator...

jyang’s picture

It also happens when I click forum menu in my primary links when I am not admin.

killes@www.drop.org’s picture

you use some kind of node access module. Which one?

jyang’s picture

Thanks,I am using OG module, now I find if I disable settings-og-access control, it seems the error is gone.but I am not quite sure how "disable" will affect other stuff.

Julien PHAM’s picture

I'm using CCK module and taxonomy access control.
It seems the issue is about the last comment stuff. It does not appear if I'm anonymous. In a forum I have posted a message, if I login as admin I can see "last comment by XXX on YYY", but that does not appear if I'm not logged in. If I'm anonymous or even as a simple user without some rights, I have the SQL errors, and I see N/A in the last comment column.

btw I'm using MySQL 5.

Julien PHAM’s picture

I tried disabling CCK but I still have the error...

moshe weitzman’s picture

Status: Active » Closed (duplicate)

lets move discussion to http://drupal.org/node/51850. i am pretty sure is same issue. also a critical.

kenorb’s picture

Version: x.y.z » 6.x-dev
Component: forum.module » node.module