* Needless to say, it's really not a good idea to use a non-unique index
* for this! That won't page right.
*
- * @return string|array
+ * @return string|string[]
*/
abstract function getIndexField();
* page_len,page_id avoids temp tables (given a page_len index). This would
* also work if page_id was non-unique but we had a page_len,page_id index.
*
- * @return array
+ * @return string[]|array[]
*/
protected function getExtraSortFields() {
return [];
}
$this->getDateRangeCond( $startTimestamp, $endTimestamp );
- // This property on IndexPager is set by $this->getIndexField() in parent::__construct().
- // We need to reassign it here so that it is used when the actual query is ran.
- $this->mIndexField = $this->getIndexField();
-
// Most of this code will use the 'contributions' group DB, which can map to replica DBs
// with extra user based indexes or partioning by user. The additional metadata
// queries should use a regular replica DB since the lookup pattern is not all by user.
$ipRangeConds = $user->isAnon() ? $this->getIpRangeConds( $this->mDb, $this->target ) : null;
if ( $ipRangeConds ) {
$queryInfo['tables'][] = 'ip_changes';
+ /**
+ * These aliases make `ORDER BY rev_timestamp, rev_id` from {@see getIndexField} and
+ * {@see getExtraSortFields} use the replicated `ipc_rev_timestamp` and `ipc_rev_id`
+ * columns from the `ip_changes` table, for more efficient queries.
+ * @see https://phabricator.wikimedia.org/T200259#4832318
+ */
+ $queryInfo['fields'] = array_merge(
+ [
+ 'rev_timestamp' => 'ipc_rev_timestamp',
+ 'rev_id' => 'ipc_rev_id',
+ ],
+ array_diff( $queryInfo['fields'], [
+ 'rev_timestamp',
+ 'rev_id',
+ ] )
+ );
$queryInfo['join_conds']['ip_changes'] = [
'LEFT JOIN', [ 'ipc_rev_id = rev_id' ]
];
}
/**
- * Override of getIndexField() in IndexPager.
- * For IP ranges, it's faster to use the replicated ipc_rev_timestamp
- * on the `ip_changes` table than the rev_timestamp on the `revision` table.
- * @return string Name of field
+ * @return string
*/
public function getIndexField() {
- if ( $this->isQueryableRange( $this->target ) ) {
- return 'ipc_rev_timestamp';
- } else {
- return 'rev_timestamp';
- }
+ // Note this is run via parent::__construct() *before* $this->target is set!
+ return 'rev_timestamp';
+ }
+
+ /**
+ * @return string[]
+ */
+ protected function getExtraSortFields() {
+ // Note this is run via parent::__construct() *before* $this->target is set!
+ return [ 'rev_id' ];
}
function doBatchLookups() {
<?php
+use Wikimedia\TestingAccessWrapper;
+
/**
* @group Database
*/
* @param array $inputOpts Input options
* @param array $expectedOpts Expected options
*/
- public function testDateFilterOptionProcessing( $inputOpts, $expectedOpts ) {
+ public function testDateFilterOptionProcessing( array $inputOpts, array $expectedOpts ) {
$this->assertArraySubset( $expectedOpts, ContribsPager::processDateFilter( $inputOpts ) );
}
[ '2001:db8::/9999' ],
];
}
+
+ /**
+ * @covers \ContribsPager::getExtraSortFields
+ * @covers \ContribsPager::getIndexField
+ * @covers \ContribsPager::getQueryInfo
+ */
+ public function testUniqueSortOrderWithoutIpChanges() {
+ $pager = new ContribsPager( new RequestContext(), [
+ 'start' => '',
+ 'end' => '',
+ ] );
+
+ /** @var ContribsPager $pager */
+ $pager = TestingAccessWrapper::newFromObject( $pager );
+ $queryInfo = $pager->buildQueryInfo( '', 1, false );
+
+ $this->assertNotContains( 'ip_changes', $queryInfo[0] );
+ $this->assertArrayNotHasKey( 'ip_changes', $queryInfo[5] );
+ $this->assertContains( 'rev_timestamp', $queryInfo[1] );
+ $this->assertContains( 'rev_id', $queryInfo[1] );
+ $this->assertSame( [ 'rev_timestamp DESC', 'rev_id DESC' ], $queryInfo[4]['ORDER BY'] );
+ }
+
+ /**
+ * @covers \ContribsPager::getExtraSortFields
+ * @covers \ContribsPager::getIndexField
+ * @covers \ContribsPager::getQueryInfo
+ */
+ public function testUniqueSortOrderOnIpChanges() {
+ $pager = new ContribsPager( new RequestContext(), [
+ 'target' => '116.17.184.5/32',
+ 'start' => '',
+ 'end' => '',
+ ] );
+
+ /** @var ContribsPager $pager */
+ $pager = TestingAccessWrapper::newFromObject( $pager );
+ $queryInfo = $pager->buildQueryInfo( '', 1, false );
+
+ $this->assertContains( 'ip_changes', $queryInfo[0] );
+ $this->assertArrayHasKey( 'ip_changes', $queryInfo[5] );
+ $this->assertSame( 'ipc_rev_timestamp', $queryInfo[1]['rev_timestamp'] );
+ $this->assertSame( 'ipc_rev_id', $queryInfo[1]['rev_id'] );
+ $this->assertSame( [ 'rev_timestamp DESC', 'rev_id DESC' ], $queryInfo[4]['ORDER BY'] );
+ }
+
}