Per r83812 CR, solve the categorymembers paging problem by doing separate queries...

Code was largely copied from Tim's CR comment on r83812 but adapted to deal with the fact that we have to apply the cmcontinue-induced WHERE on cl_sortkey and cl_from only for the first query. Because ApiQueryBase doesn't let us unset or overwrite conditions like these in a nice way, I added an $extraQuery parameter to ApiQueryBase::select() that excepts additional query parameters that are only applied to that query but not stored in the object.

diff --git a/includes/api/ApiQueryBase.php b/includes/api/ApiQueryBase.php
index 8171466..3fca782 100644 (file)
--- a/includes/api/ApiQueryBase.php
+++ b/includes/api/ApiQueryBase.php
@@ -246,14 +246,21 @@ abstract class ApiQueryBase extends ApiBase {
         * Execute a SELECT query based on the values in the internal arrays
         * @param $method string Function the query should be attributed to.
         *  You should usually use __METHOD__ here
+        * @param $extraQuery array Query data to add but not store in the object
+        *  Format is array( 'tables' => ..., 'fields' => ..., 'where' => ..., 'options' => ..., 'join_conds' => ... )
         * @return ResultWrapper
         */
-       protected function select( $method ) {
+       protected function select( $method, $extraQuery = array() ) {
+               // Merge $this->tables with $extraQuery['tables'], $this->fields with $extraQuery['fields'], etc.
+               foreach ( array( 'tables', 'fields', 'where', 'options', 'join_conds' ) as $var ) {
+                       $$var = array_merge( $this->{$var}, isset( $extraQuery[$var] ) ? (array)$extraQuery[$var] : array() );
+               }
+               
                // getDB has its own profileDBIn/Out calls
                $db = $this->getDB();
 
                $this->profileDBIn();
-               $res = $db->select( $this->tables, $this->fields, $this->where, $method, $this->options, $this->join_conds );
+               $res = $db->select( $tables, $fields, $where, $method, $options, $join_conds );
                $this->profileDBOut();
 
                return $res;

diff --git a/includes/api/ApiQueryCategoryMembers.php b/includes/api/ApiQueryCategoryMembers.php
index e2d0149..5f249b2 100644 (file)
--- a/includes/api/ApiQueryCategoryMembers.php
+++ b/includes/api/ApiQueryCategoryMembers.php
@@ -99,7 +99,8 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
                $this->addTables( array( 'page', 'categorylinks' ) );   // must be in this order for 'USE INDEX'
 
                $this->addWhereFld( 'cl_to', $categoryTitle->getDBkey() );
-               $this->addWhereFld( 'cl_type', $params['type'] );
+               $queryTypes = $params['type'];
+               $contWhere = false;
 
                // Scanning large datasets for rare categories sucks, and I already told
                // how to have efficient subcategory access :-) ~~~~ (oh well, domas)
@@ -129,20 +130,22 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
                                                'by the previous query', '_badcontinue'
                                        );
                                }
-                               $escType = $this->getDB()->addQuotes( $cont[0] );
+                               
+                               // Remove the types to skip from $queryTypes
+                               $contTypeIndex = array_search( $cont[0], $queryTypes );
+                               $queryTypes = array_slice( $queryTypes, $contTypeIndex );
+                               
+                               // Add a WHERE clause for sortkey and from
                                $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 " .
+                               // $contWhere is used further down
+                               $contWhere = "cl_sortkey $op $escSortkey OR " .
                                        "(cl_sortkey = $escSortkey AND " .
-                                       "cl_from $op= $from)))"
-                               );
+                                       "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 );
+                               // The below produces ORDER BY cl_sortkey, cl_from, possibly with DESC added to each of them
                                $this->addWhereRange( 'cl_sortkey',
                                        $dir,
                                        $params['startsortkey'],
@@ -157,9 +160,29 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
                $limit = $params['limit'];
                $this->addOption( 'LIMIT', $limit + 1 );
 
+               // Run a separate SELECT query for each value of cl_type.
+               // This is needed because cl_type is an enum, and MySQL has
+               // inconsistencies between ORDER BY cl_type and
+               // WHERE cl_type >= 'foo' making proper paging impossible
+               // and unindexed.
+               $rows = array();
+               $first = true;
+               foreach ( $queryTypes as $type ) {
+                       $extraConds = array( 'cl_type' => $type );
+                       if ( $first && $contWhere ) {
+                               // Continuation condition. Only added to the
+                               // first query, otherwise we'll skip things
+                               $extraConds[] = $contWhere;
+                       }
+                       $res = $this->select( __METHOD__, array( 'where' => $extraConds ) );
+                       $rows = array_merge( $rows, iterator_to_array( $res ) );
+                       if ( count( $rows ) >= $limit + 1 ) {
+                               break;
+                       }
+                       $first = false;
+               }
                $count = 0;
-               $res = $this->select( __METHOD__ );
-               foreach ( $res as $row ) {
+               foreach ( $rows as $row ) {
                        if ( ++ $count > $limit ) {
                                // We've reached the one extra which shows that there are additional pages to be had. Stop here...
                                // TODO: Security issue - if the user has no right to view next title, it will still be shown