Fixed Possible non-deterministic sorting in findNextPostsInThread/findNewestPostsInThread

[Xon]

Affected version

2.1.10 Patch 1

Both findNextPostsInThread /findNewestPostsInThread had their sort criteria changed, but switched to a single sort key without any uniqueness constraint.

Previously it was position, postdate, but is now just post_date, without a sort criteria for when post_date values are equal.

While it is technically very unlikely to have two posts with the same post_date, it isn't prevented, and in that case these functions will have potentially non-deterministic sorting.

 

[Chris D]

This was changed as a result of your bug report where you explicitly suggested changing it from ['position', 'post_date'] to just post_date.

Fixed - Unexpectedly expensive query in Thread::actionNewPosts for long threads

In XF\Pub\Controller\Thread::actionNewPosts (and a few other 'next-post' like methods), there is a call to findNewestPostsInThread (this applies to findNextPostsInThread) which finds the next (or previous) post by post date. this query can be unexpectedly expensive for long threads due to the...

xenforo.com

Did you have an alternative solution in mind when you reported that?

 

[Xon]

I see my hot-fix was just the one column sort, ugh

Post_date and post_id as sort columns should be safe as that is how the the post position is determined when rebuilding the thread. (Desc vs asc as required).

 

[Xon]

Running the two queries;

Spoiler: with out post_id

Analyze format=json
SELECT `xf_post`.*
FROM `xf_post`
WHERE (`xf_post`.`thread_id` = 44838) AND ((`xf_post`.`message_state` = 'moderated' AND `xf_post`.`user_id` = 20190) OR (`xf_post`.`message_state` IN ('visible'))) AND (`xf_post`.`post_date` > 1570145112)
ORDER BY `xf_post`.`post_date` DESC
LIMIT 4;

{
    "select_id": 1,
    "r_loops": 1,
    "r_total_time_ms": 1.0564,
    "table": {
      "table_name": "xf_post",
      "access_type": "range",
      "possible_keys": [
        "thread_id_position",
        "user_id",
        "thread_id_post_date",
        "post_date"
      ],
      "key": "thread_id_post_date",
      "key_length": "8",
      "used_key_parts": ["thread_id", "post_date"],
      "r_loops": 1,
      "rows": 1,
      "r_rows": 0,
      "r_total_time_ms": 1.035,
      "filtered": 100,
      "r_filtered": 100,
      "attached_condition": "xf_post.thread_id = 44838 and (xf_post.user_id = 20190 and xf_post.message_state = 'moderated' or xf_post.message_state = 'visible') and xf_post.post_date > 1570145112"
    }
  }
}

Spoiler: with post_id

Analyze format=json
SELECT `xf_post`.*
FROM `xf_post`
WHERE (`xf_post`.`thread_id` = 44838) AND ((`xf_post`.`message_state` = 'moderated' AND `xf_post`.`user_id` = 20190) OR (`xf_post`.`message_state` IN ('visible'))) AND (`xf_post`.`post_date` > 1570145112)
ORDER BY `xf_post`.`post_date` DESC, `xf_post`.`post_id` DESC
LIMIT 4;

{
  "query_block": {
    "select_id": 1,
    "r_loops": 1,
    "r_total_time_ms": 1.1129,
    "table": {
      "table_name": "xf_post",
      "access_type": "range",
      "possible_keys": [
        "thread_id_position",
        "user_id",
        "thread_id_post_date",
        "post_date"
      ],
      "key": "thread_id_post_date",
      "key_length": "8",
      "used_key_parts": ["thread_id", "post_date"],
      "r_loops": 1,
      "rows": 1,
      "r_rows": 0,
      "r_total_time_ms": 1.0922,
      "filtered": 100,
      "r_filtered": 100,
      "attached_condition": "xf_post.thread_id = 44838 and (xf_post.user_id = 20190 and xf_post.message_state = 'moderated' or xf_post.message_state = 'visible') and xf_post.post_date > 1570145112"
    }
  }
}

(Both cold queries against the same database)

Same resulting query plan, but the second will prevent joins added by add-ons doing something crazy to the sorting.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.0 Beta 2).

Change log:

Fix potential non-deterministic order when fetching next/newest posts in a thread.

There may be a delay before changes are rolled out to the XenForo Community.