From 64d9832d1f19f4daca80910a62d309785b2d5571 Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Sun, 13 Mar 2011 10:39:57 +0000 Subject: [PATCH] (bug 27965) Paging in list=categorymembers was completely broken. It was paging by cl_from alone, while the index is on (cl_to, cl_type, cl_sortkey, cl_from) and only cl_to is constant. Fixed by paging on (type, sortkey, from), but using type|from|sortkey for clcontinue so any pipe characters in the sortkey are easier to handle. This needs the schema change in r83812 to work correctly, otherwise rows with cl_type=file will be skipped in certain cases. --- includes/api/ApiQueryCategoryMembers.php | 73 ++++++++++++++---------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/includes/api/ApiQueryCategoryMembers.php b/includes/api/ApiQueryCategoryMembers.php index d1b4d8bfc2..1ecacd717b 100644 --- a/includes/api/ApiQueryCategoryMembers.php +++ b/includes/api/ApiQueryCategoryMembers.php @@ -86,17 +86,15 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { $fld_type = isset( $prop['type'] ); if ( is_null( $resultPageSet ) ) { - $this->addFields( array( 'cl_from', 'page_namespace', 'page_title' ) ); + $this->addFields( array( 'cl_from', 'cl_sortkey', 'cl_type', 'page_namespace', 'page_title' ) ); $this->addFieldsIf( 'page_id', $fld_ids ); $this->addFieldsIf( 'cl_sortkey_prefix', $fld_sortkeyprefix ); - $this->addFieldsIf( 'cl_sortkey', $fld_sortkey ); } else { $this->addFields( $resultPageSet->getPageTableFields() ); // will include page_ id, ns, title - $this->addFields( array( 'cl_from', 'cl_sortkey' ) ); + $this->addFields( array( 'cl_from', 'cl_sortkey', 'cl_type' ) ); } $this->addFieldsIf( 'cl_timestamp', $fld_timestamp || $params['sort'] == 'timestamp' ); - $this->addFieldsIf( 'cl_type', $fld_type ); $this->addTables( array( 'page', 'categorylinks' ) ); // must be in this order for 'USE INDEX' @@ -123,18 +121,37 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { $this->addOption( 'USE INDEX', 'cl_timestamp' ); } else { - // The below produces ORDER BY cl_type, cl_sortkey, cl_from, possibly with DESC added to each of them - $this->addWhereRange( 'cl_type', $dir, null, null ); - $this->addWhereRange( 'cl_sortkey', - $dir, - $params['startsortkey'], - $params['endsortkey'] ); - $this->addWhereRange( 'cl_from', $dir, null, null ); + if ( $params['continue'] ) { + // type|from|sortkey + $cont = explode( '|', $params['continue'], 3 ); + if ( count( $cont ) != 3 ) { + $this->dieUsage( 'Invalid continue param. You should pass the original value returned '. + 'by the previous query', '_badcontinue' + ); + } + $escType = $this->getDB()->addQuotes( $cont[0] ); + $from = intval( $cont[1] ); + $escSortkey = $this->getDB()->addQuotes( $cont[2] ); + $op = $dir == 'newer' ? '>' : '<'; + $this->addWhere( "cl_type $op $escType OR " . + "(cl_type = $escType AND " . + "(cl_sortkey $op $escSortkey OR " . + "(cl_sortkey = $escSortkey AND " . + "cl_from $op= $from)))" + ); + + } else { + // The below produces ORDER BY cl_type, cl_sortkey, cl_from, possibly with DESC added to each of them + $this->addWhereRange( 'cl_type', $dir, null, null ); + $this->addWhereRange( 'cl_sortkey', + $dir, + $params['startsortkey'], + $params['endsortkey'] ); + $this->addWhereRange( 'cl_from', $dir, null, null ); + } $this->addOption( 'USE INDEX', 'cl_sortkey' ); } - $this->setContinuation( $params['continue'], $params['dir'] ); - $this->addWhere( 'cl_from=page_id' ); $limit = $params['limit']; @@ -149,7 +166,13 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { if ( $params['sort'] == 'timestamp' ) { $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->cl_timestamp ) ); } else { - $this->setContinueEnumParameter( 'continue', $row->cl_from ); + // Continue format is type|from|sortkey + // The order is a bit weird but it's convenient to put the sortkey at the end + // because we don't have to worry about pipes in the sortkey that way + // (and type and from can't contain pipes anyway) + $this->setContinueEnumParameter( 'continue', + "{$row->cl_type}|{$row->cl_from}|{$row->cl_sortkey}" + ); } break; } @@ -189,7 +212,9 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { if ( $params['sort'] == 'timestamp' ) { $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->cl_timestamp ) ); } else { - $this->setContinueEnumParameter( 'continue', $row->cl_from ); + $this->setContinueEnumParameter( 'continue', + "{$row->cl_type}|{$row->cl_from}|{$row->cl_sortkey}" + ); } break; } @@ -204,21 +229,6 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { } } - /** - * Add DB WHERE clause to continue previous query based on 'continue' parameter - */ - private function setContinuation( $continue, $dir ) { - if ( is_null( $continue ) ) { - return; // This is not a continuation request - } - - $encFrom = $this->getDB()->addQuotes( intval( $continue ) ); - - $op = ( $dir == 'desc' ? '<=' : '>=' ); - - $this->addWhere( "cl_from $op $encFrom" ); - } - public function getAllowedParams() { return array( 'title' => array( @@ -316,7 +326,8 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { $desc['namespace'] = array( $desc['namespace'], 'NOTE: Due to $wgMiserMode, using this may result in fewer than "limit" results', - 'returned before continuing; in extreme cases, zero results may be returned', + 'returned before continuing; in extreme cases, zero results may be returned.', + 'Note that you can use cmtype=subcat or cmtype=file instead of cmnamespace=14 or 6.', ); } return $desc; -- 2.20.1