This index doesn't make sense, and almost everything that uses it is a bug.
The correct ordering of revisions within a page is by timestamp, except where there are duplicate timestamps, in which case rev_id is used to break the tie.
rev_id does not increase monotonically with rev_timestamp, so it's not appropriate to use it for revision ordering within a page. It can be assumed to reflect time only when rev_timestamp matches.
Let's consider the callers.
ApiQueryRevisions has startid/endid parameters. These can be mapped to a (rev_timestamp,rev_id) tuple in the proposed total order before performing the query. If an unknown rev_id is requested as an endpoint, it can fail. Currently startid/endid is broken, since it partitions by rev_id but sorts by rev_timestamp, and so causes queries to take tens of seconds (T163495).
Title::getNextRevisionID() gets the next revision, ordering by ID, getPreviousRevisionID() is the same in the opposite direction. The original use case for these functions were the prev/next links on the old revision page views. The user certainly expects a time-based ordering in that case. The other callers:
- WatchedItemStore::getNotificationTimestamp() uses it to check if a given rev_id is the latest, which is totally broken. It should use page_latest. This is duplicated in ApiSetNotificationTimestamp and User::clearNotification().
- Article::view() has a direction parameter, next or prev, which was introduced to avoid the need for an expensive getNextRevisionID() call on old revision page views. As mentioned above, this should use time-based ordering.
- Article::setOldSubtitle() calls getPreviousRevisionID() to check if the "previous" link needs to be displayed on an old revision view, which rather defeats the efficiency purpose of the direction parameter. rev_parent_id could be used, avoiding the need for a DB query, assuming the column is fully populated.
- RawAction has a direction parameter analogous to the action=view one, which should function in the same way as action=view.
- The "undo" feature in EditPage and duplicated in ApiEditPage has a range of revisions specified by the rev_id of the start and end of the range. It uses Revision::getNext() to determine whether the start and end are consecutive revisions. This should use the time-based total order or rev_parent_id.
- DifferenceEngine has a diff=prev, diff=next parameters and uses getNextRevisionID()/getPreviousRevisionID() to interpret these parameters. Like direction parameters, this would benefit from using a time-based order. HistoryAction::feedItem() uses getPreviousRevisionID() similarly.
- CategoryMembershipChange uses getPreviousRevisionID() to get the timestamp for sending $lastTimestamp to RecentChange. It should ideally use the pre-update page_latest value like we do on page save.
{T159319} would be fixed by making this proposed change.
Searching for SQL with rev_id inequalities:
- dumpBackup.php has --revrange, possibly a legitimate use case, but could be answered by scanning the table.
- compressOld.php uses rev_id<page_latest, it should use rev_id<>page_latest instead.
- Already using the proposed total order: populateParentId.php, MysqlUpdater::doSchemaRestructuring(), CategoryMembershipChangeJob.
