From: Marcin Cieślak Date: Tue, 15 Mar 2011 02:53:00 +0000 (+0000) Subject: r83812, r83814: Don't use cl_type at all when paging categorylinks X-Git-Tag: 1.31.0-rc.0~31398 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/password.php?a=commitdiff_plain;h=274d317a0ca2777a5c5fae2007c53811397540f5;p=lhc%2Fweb%2Fwiklou.git 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 e2d0149ed6..7ca03222a8 100644 --- 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 db9f9c5944..c3186f21c8 100644 --- 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 index cf7b9c3a1a..0000000000 --- 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 99b70e4169..11392f1855 100644 --- 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);