r83812, r83814: Don't use cl_type at all when paging categorylinks

* Remove cl_type from paging in categorylinks - it's not
  really needed there. Although cl_type is in WHERE but not
  in ORDER BY clause the worst thing that can happen
  is to have a filesort going again through <500 entries
  selected by index. Or will FORCE INDEX work anyway?

* Revert schema change, as we don't need cl_type there
  anyway (or even if we wanted to compare, it should
  work as expected by using INT values against ENUM).

diff --git a/includes/api/ApiQueryCategoryMembers.php b/includes/api/ApiQueryCategoryMembers.php
index e2d0149..7ca0322 100644 (file)
--- a/includes/api/ApiQueryCategoryMembers.php
+++ b/includes/api/ApiQueryCategoryMembers.php
@@ -122,27 +122,39 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
                        $this->addOption( 'USE INDEX', 'cl_timestamp' );
                } else {
                        if ( $params['continue'] ) {
-                               // type|from|sortkey
-                               $cont = explode( '|', $params['continue'], 3 );
-                               if ( count( $cont ) != 3 ) {
+                               // from|sortkey
+                               $cont = explode( '|', $params['continue'], 2 );
+                               if ( count( $cont ) != 2 ) {
                                        $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)))"
-                               );
+                               list ( $from, $contsortkey )  = $cont;
+                               if ( intval( $from ) == 0 ) {
+                                       $this->dieUsage( 'Invalid continue param. You should pass the original value returned '.
+                                               'by the previous query', '_badcontinue'
+                                       );
+                               }
+                               $where_outer = array();
+                               $where_inner = array();
+                               $db = $this->getDB();
+                               $op = ( $dir === 'newer' ? '>' : '<' );
+                               $sortdir = ( $dir === 'newer' ? 'asc' : 'desc' );
+                               $where_outer[] = 'cl_sortkey ' . $op . ' ' .
+                                       $db->addQuotes( $contsortkey );
+                               // OR
+                                       $where_inner[] = 'cl_sortkey = ' .
+                                               $db->addQuotes( $contsortkey );
+                                       // AND
+                                       $where_inner[] = 'cl_from ' . $op . '= '.  $from;
+
+                               $where_outer[] = $db->makeList( $where_inner, LIST_AND );
+                               $this->addWhere( $db->makeList( $where_outer, LIST_OR ) );
+                               $this->addOption( 'ORDER BY', 
+                                       'cl_sortkey ' . $sortdir .', cl_from ' . $sortdir );
                                
                        } 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'],
@@ -171,7 +183,7 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
                                        // 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}"
+                                               "{$row->cl_from}|{$row->cl_sortkey}"
                                        );
                                }
                                break;
@@ -213,7 +225,7 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
                                                $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->cl_timestamp ) );
                                        } else {
                                                $this->setContinueEnumParameter( 'continue',
-                                                       "{$row->cl_type}|{$row->cl_from}|{$row->cl_sortkey}"
+                                                       "{$row->cl_from}|{$row->cl_sortkey}"
                                                );
                                        }
                                        break;

diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php
index db9f9c5..c3186f2 100644 (file)
--- a/includes/installer/MysqlUpdater.php
+++ b/includes/installer/MysqlUpdater.php
@@ -175,7 +175,6 @@ class MysqlUpdater extends DatabaseUpdater {
                        array( 'dropIndex', 'archive', 'ar_page_revid',         'patch-archive_kill_ar_page_revid.sql' ),
                        array( 'addIndex', 'archive', 'ar_revid',               'patch-archive_ar_revid.sql' ),
                        array( 'doLangLinksLengthUpdate' ),
-                       array( 'doClTypeVarcharUpdate' ),
                );
        }
 
@@ -829,18 +828,4 @@ class MysqlUpdater extends DatabaseUpdater {
                        $this->output( "...ll_lang is up-to-date.\n" );
                }
        }
-       
-       protected function doClTypeVarcharUpdate() {
-               $categorylinks = $this->db->tableName( 'categorylinks' );
-               $res = $this->db->query( "SHOW COLUMNS FROM $categorylinks LIKE 'cl_type'" );
-               $row = $this->db->fetchObject( $res );
-               
-               if ( $row && substr( $row->Type, 0, 4 ) == 'enum' ) {
-                       $this->output( 'Changing cl_type from enum to varchar...' );
-                       $this->applyPatch( 'patch-cl_type.sql' );
-                       $this->output( "done.\n" );
-               } else {
-                       $this->output( "...cl_type is up-to-date.\n" );
-               }
-       }
 }

diff --git a/maintenance/archives/patch-cl_type.sql b/maintenance/archives/patch-cl_type.sql
deleted file mode 100644 (file)
index cf7b9c3..0000000
--- a/maintenance/archives/patch-cl_type.sql
+++ /dev/null
@@ -1,6 +0,0 @@
---
--- Change cl_type to a varchar from an enum because of the weird semantics of
--- the < and > operators when working with enums
---
-
-ALTER TABLE /*_*/categorylinks MODIFY cl_type varchar(6) NOT NULL default 'page';

diff --git a/maintenance/tables.sql b/maintenance/tables.sql
index 99b70e4..11392f1 100644 (file)
--- a/maintenance/tables.sql
+++ b/maintenance/tables.sql
@@ -521,9 +521,7 @@ CREATE TABLE /*_*/categorylinks (
   -- paginate the three categories separately.  This never has to be updated
   -- after the page is created, since none of these page types can be moved to
   -- any other.
-  -- This used to be ENUM('page', 'subcat', 'file') but was changed to a
-  -- varchar because of the weird semantics of < and > when used on enums
-  cl_type varchar(6) NOT NULL default 'page'
+  cl_type ENUM('page', 'subcat', 'file') NOT NULL default 'page'
 ) /*$wgDBTableOptions*/;
 
 CREATE UNIQUE INDEX /*i*/cl_from ON /*_*/categorylinks (cl_from,cl_to);