I'm not sure if this is the appropriate place to post this bug, so appologies in advance if this is the wrong place.

It only seems to be apparent when using the node_privacy_byrole.module, which I realize is not core. However, I'm not sure the problem is really due to the contributed module, rather than the unexpected way it interacts with 4.5.0

Here's what's going on: When using the node_privacy_byrole.module, it appears that for each gid (either a user id or a role id, depending on whether its node_privacy_byrole_user or node_privacy_byrole_role) with grant_view = 1 in the node_access table for a particular nid, that node will be displayed multiple times for each gid applicable to a given user.

For example, if a user is assigned 'role1' and 'role2', and both of those roles are given view access to a node, that node will be displayed twice for that user (at least if the content type is a forum topic, or an event in the event.module. Not sure this occurs for pages.)

Also, if a user is assigned 'role1' and both node_privacy_byrole_role for that role id is granted view access, and node_privacy_byrole_user for that user id is granted view access, once again, the node will be displayed twice.

Again, if a user has more than one access granted to a node, either through user and role access, or multiple role access, the node will be displayed for that user 1 time per access grant.

Here's an example of what's in the node_access table:

nid   gid   realm                        grant_view  grant_update  grant_delete
25    3     node_privacy_byrole_role     1           0             0
25    4     node_privacy_byrole_role     1           0             0
25    5     node_privacy_byrole_role     1           0             0
25    2     node_privacy_byrole_user     1           0             0
26    1     node_privacy_byrole_role     1           0             0
26    1     node_privacy_byrole_user     1           0             0

For node 25, user_id 2, as well as role_id's 3, 4, and 5, all have grant_view = 1. When user_id 2 is logged in, and assuming user_id 2 has roles 3, 4, and 5 assigned, that forum topic will show up 4 times, and the number indicating the number of posts to that forum, and the number of new posts, will be incremented by 4 rather than 1.

Similarly for node 26, user_id 1 as well as role_id 1 have grant_view = 1. When user_id 1 is logged in, and assuming user_id 1 has roles 1 assigned, that forum topic will show up 2 times, and the number indicating the number of posts to that forum, and the number of new posts, will be incremented by 2 rather than 1.

I'm not sure where the correct place to post this bug would be. It's not really a forum.module bug, as it only relates to the usage of a contributed module, but it seems to be a problem with what is being queried from the database.

This doesn't only show up for forum topic content, but I'm not sure what it is limited to. It doesn't seem to be a problem for pages, but it is if using the event.module for events.

Any help in trying to track down what's going on would be appreciated.

See also:

http://drupal.org/node/11719
http://drupal.org/node/11749

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JonBob’s picture

I think maybe the forum module lost a "DISTINCT" in its SQL queries when the speedup patches were being formed. Line 420 seems to need one, for example. No time to cook a patch at the moment, but I might get to it eventually if nobody steps up.

stelman’s picture

Title: Forum topics being displayed multiple times » You are correct

Yep, that seems to be the problem! I added the one that you mentioned, and that solved part of the problem. The topic count is still off, but they are only displayed once now.

I believe there are similar problems elsewhere in the code. As I mentioned, a similar problem seems to occur when using the event.module.

stelman’s picture

Title: You are correct » Forum topics being displayed multiple times

oops - replacing title

killes@www.drop.org’s picture

Assigned: Unassigned » killes@www.drop.org
FileSize
2.13 KB

Here is a patch that adds DISTINCT to n.nid.

JonBob’s picture

I think we need a matching DISTINCT in the count, or the pager will be off. Also, this patch seems to contain an unrelated forum_access() change.

killes@www.drop.org’s picture

FileSize
2.01 KB

Patch corrected.

killes@www.drop.org’s picture

stelman’s picture

Thank you! But still not completely fixed. If the forum topics block is enabled, duplicates still show up in the "Active Forum Topics:" and "New Forum Topics:" lists.

I don't really know PHP, or how to make patches properly, but looking through the code, the following changes need to be made in the function forum_block:

lines 124 and 126 (approximately) need DISTINCT added also:

      $content  = node_title_list(db_query_range("SELECT DISTINCT(n.nid), n.title, .....

      $content .= node_title_list(db_query_range("SELECT DISTINCT(n.nid), n.title, ..... 

So, then I question whether the other few locations that still have "SELECT n.nid" in the code (3 places, I think) also need to be changed to "SELECT DISTINCT(n.nid)"? I haven't figured out yet exactly what's going on there.

I appreciate your help.

killes@www.drop.org’s picture

FileSize
7.12 KB

Updated.

moshe weitzman’s picture

at first glance, it seems that all node listings need this DISTINCT() feature (home page, blog page, etc.)

Dries’s picture

Committed to HEAD and DRUPAL-4-5.

Dries’s picture

This patch doubled the execution time of pages that display the forum block.

Steve Dondley’s picture

OK, so this fixes the forum module. What about the problem node_privacy_byrols is causing by creating duplicate content in other blocks? For example, for weblinks and events, if a user has two roles, the links show up twice in the block.

I know these aren't part of the core. But is anyone working on this issue?

Anonymous’s picture

For what it's worth, and as suggested above, I followed the spirit of killes@www.drop.org's patch and fixed the related events.module problem I was having (the forum.module problem too, of course). I added "DISTINCT ()" after the "SELECT" on line 1074 of the events.module and all is well now. This seemed to be the only place within events.module where the DISTINCT was missing.

Interestingly, and seeminly opposite what others were saying, I was getting duplicate posts in events and forums not when a user had two roles, but for users with anonymous or authenticate roles only. Users with multiple roles saw only one listing in the forums and events blocks. Anyway, the DISTINCT patch seems to have solved this for now. I can't speak for the other problem areas.

degerrit’s picture

I was having a similar problem with my own module.

I think 'GROUP BY n.nid' added at the end of line 420 is better than a DISTINCT (and should be at least as fast). I believe distinct will try to find a unique combination of all 'n.nid, f.tid, n.title, n.sticky, u.name, u.uid, n.created' regardless where you put the brackets, which isn't really necessary.

For performance perhaps the old query can still be used when the 'node_privacy_byrole' is disabled?

moshe weitzman’s picture

I always wondered what the DB did with that DISTINCT statement. Does it take the first of duplicate rows? You are right that a GROUP BY would be more explicit. We should get this right before the node query patch lands.

degerrit’s picture

My statement was wrong in this case; if the DISTINCT is followed by an ON statent, that's the column which is made unique otherwise DISTINCT will eliminate duplicate rows, so essentially here it is the same as GROUP BY.

Anyway, GROUP BY nid is still more transparent about what the query is trying to achieve, perhaps the performance issue noted can be solved by doing two separate queries or a subquery. I'll see if I can come up with something, although my 3-topic forum is not a very good test case ;-)

killes@www.drop.org’s picture

Status: Active » Fixed

the original issue was fixed.

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Status: Fixed » Closed (fixed)