As designed DB errors due to missing table join

[veraderock]

In tracking down a database error, I've stumbled on a bug in the following location:

File: library/XenForo/Model/ThreadWatch.php
Function declaration: getThreadsWatchedByUser
Line #: 311
Issue:
If $newOnly is true then the $newOnlyClause includes a comparison against the tables 'thread_read' and 'forum_read', however there is no addition of those tables to the $joinOptions['joinTables'] . So, the resulting SQL statement fails.

 

[Chris D]

I can't see the bug here.

A $userId must be passed to that function which sets $fetchOptions['readUserId'] to that user ID. When the thread fetch options are prepared if there is a readUserId key (which there always will be with this function), then a join is made to the thread_read and forum_read tables.

 

[Mike]

The only way this could happen is if no user ID was passed in, which is a required value in this instance as it's nonsensical without it (there is no method for tracking what's new to a guest). Similarly, there's no concept of a guest watching a thread, so the fundamental query itself doesn't make sense for a guest.

As such, I would consider this expected.

 

[veraderock]

Ok, well I stumbled across this by sending a copy of the attempted SQL error to my email on a fail, this was due to a totally unrelated issue and it did help me correct the other issue. But in having that setup I found I received a lot of emails with the following content:

SELECT thread.*,
                    thread_watch.email_subscribe
                    ,
                    user.*, IF(user.username IS NULL, thread.username, user.username) AS username,
                    node.title AS node_title, node.node_name,
                    NULL AS thread_read_date,
                    0 AS user_post_count,
                permission.cache_value AS node_permission_cache,
                last_post_user.avatar_date AS last_post_user_avatar_date, last_post_user.gravatar AS last_post_user_gravatar, last_post_user.gender AS last_post_user_gender
                FROM xf_thread_watch AS thread_watch
                INNER JOIN xf_thread AS thread ON
                    (thread.thread_id = thread_watch.thread_id)
              
                    LEFT JOIN xf_user AS user ON
                        (user.user_id = thread.user_id)
                    LEFT JOIN xf_node AS node ON
                        (node.node_id = thread.node_id)
                LEFT JOIN xf_permission_cache_content AS permission
                    ON (permission.permission_combination_id = 1
                        AND permission.content_type = 'node'
                        AND permission.content_id = thread.node_id)
                LEFT JOIN xf_user AS last_post_user ON
                    (last_post_user.user_id = thread.last_post_user_id)
                WHERE thread_watch.user_id = ?
                    AND thread.discussion_state = 'visible'
                  
                AND thread.last_post_date > 1411933660
                AND thread.last_post_date > COALESCE(thread_read.thread_read_date, 0)
                AND thread.last_post_date > COALESCE(forum_read.forum_read_date, 0)
          
                ORDER BY thread.last_post_date DESC

This was on a version 1.3.5 install, however I posted the error only because I see the same function declaration in 1.4.2 with no changes.. I haven't setup the same email notification in a 1.4.2 install so it's entirely possible it's no longer an issue. But thought it worth bringing to your attention.

 

[Mike]

Based on the "NULL AS thread_read_date" portion, that's what happens when you pass no user ID in.

 

[veraderock]

So in that case, I guess the question is what triggers that situation.
Although I haven't seen that error in the server error logs, it only became evident when sending the failed SQL call to my email. I'm not aware of it interfering with any other functionality.

 

[Mike]

Well, there's only one place in XF that triggers this query and it is not possible to reach it without being logged in. That could imply it's an add-on triggering it.