(bug 36400) API: Fix sorting for iwlinks, langlinks

The iwlinks and langlinks modules continue parameters imply ordering by
page then prefix then title. But in certain modes, the actual queries
use a different ordering, which may result in skipped or repeated
results.

This changeset fixes that. To do so, it needs to re-add an index
iwl_prefix_from_title which was mistakenly removed in 2010 (r69721). And
while it's doing that, it cleans up errors in the sqlite and postgresql
handling of the iwlinks indexes too.

Also, per Asher, make the iwl_prefix_from_title and
iwl_prefix_title_from indexes non-UNIQUE.

Change-Id: I607e8bf9183a2d8152a6127a81c83a0b5bba0c61
This commit is contained in:
Brad Jorsch 2013-01-10 19:22:03 -05:00
parent cdb562b09e
commit c013ec02b9
14 changed files with 50 additions and 38 deletions

View file

@ -81,8 +81,8 @@ class ApiQueryIWLinks extends ApiQueryBase {
$this->addOption( 'ORDER BY', 'iwl_from' . $sort );
} else {
$this->addOption( 'ORDER BY', array(
'iwl_title' . $sort,
'iwl_from' . $sort
'iwl_from' . $sort,
'iwl_title' . $sort
));
}
} else {
@ -92,7 +92,8 @@ class ApiQueryIWLinks extends ApiQueryBase {
} else {
$this->addOption( 'ORDER BY', array (
'iwl_from' . $sort,
'iwl_prefix' . $sort
'iwl_prefix' . $sort,
'iwl_title' . $sort
));
}
}

View file

@ -67,18 +67,15 @@ class ApiQueryLangLinks extends ApiQueryBase {
);
}
$sort = ( $params['dir'] == 'descending' ? ' DESC' : '' );
if ( isset( $params['lang'] ) ) {
// Note that, since (ll_from, ll_lang) is a unique key, we don't need
// to sort by ll_title to ensure deterministic ordering.
$sort = ( $params['dir'] == 'descending' ? ' DESC' : '' );
if ( isset( $params['lang'] ) ) {
$this->addWhereFld( 'll_lang', $params['lang'] );
if ( isset( $params['title'] ) ) {
$this->addWhereFld( 'll_title', $params['title'] );
$this->addOption( 'ORDER BY', 'll_from' . $sort );
} else {
$this->addOption( 'ORDER BY', array(
'll_title' . $sort,
'll_from' . $sort
));
}
$this->addOption( 'ORDER BY', 'll_from' . $sort );
} else {
// Don't order by ll_from if it's constant in the WHERE clause
if ( count( $this->getPageSet()->getGoodTitles() ) == 1 ) {

View file

@ -179,7 +179,6 @@ class MysqlUpdater extends DatabaseUpdater {
array( 'addField', 'updatelog', 'ul_value', 'patch-ul_value.sql' ),
array( 'addField', 'interwiki', 'iw_api', 'patch-iw_api_and_wikiid.sql' ),
array( 'dropIndex', 'iwlinks', 'iwl_prefix', 'patch-kill-iwl_prefix.sql' ),
array( 'dropIndex', 'iwlinks', 'iwl_prefix_from_title', 'patch-kill-iwl_pft.sql' ),
array( 'addField', 'categorylinks', 'cl_collation', 'patch-categorylinks-better-collation.sql' ),
array( 'doClFieldsUpdate' ),
array( 'doCollationUpdate' ),
@ -230,6 +229,10 @@ class MysqlUpdater extends DatabaseUpdater {
array( 'modifyField', 'user_former_groups', 'ufg_group', 'patch-ufg_group-length-increase-255.sql' ),
array( 'addIndex', 'page_props', 'pp_propname_page', 'patch-page_props-propname-page-index.sql' ),
array( 'addIndex', 'image', 'img_media_mime', 'patch-img_media_mime-index.sql' ),
// 1.22
array( 'doIwlinksIndexNonUnique' ),
array( 'addIndex', 'iwlinks', 'iwl_prefix_from_title', 'patch-iwlinks-from-title-index.sql' ),
);
}
@ -902,4 +905,18 @@ class MysqlUpdater extends DatabaseUpdater {
$this->applyPatch( 'patch-user-newtalk-timestamp-null.sql', false, "Making user_last_timestamp nullable" );
}
protected function doIwlinksIndexNonUnique() {
$info = $this->db->indexInfo( 'iwlinks', 'iwl_prefix_title_from' );
if ( is_array( $info ) && $info[0]->Non_unique ) {
$this->output( "...iwl_prefix_title_from index is already non-UNIQUE.\n" );
return true;
}
if ( $this->skipSchema ) {
$this->output( "...skipping schema change (making iwl_prefix_title_from index non-UNIQUE).\n" );
return false;
}
return $this->applyPatch( 'patch-iwl_prefix_title_from-non-unique.sql', false, "Making iwl_prefix_title_from index non-UNIQUE" );
}
}

View file

@ -232,6 +232,7 @@ class PostgresUpdater extends DatabaseUpdater {
array( 'addPgIndex', 'watchlist', 'wl_user', '(wl_user)' ),
array( 'addPgIndex', 'logging', 'logging_user_type_time', '(log_user, log_type, log_timestamp)' ),
array( 'addPgIndex', 'logging', 'logging_page_id_time', '(log_page,log_timestamp)' ),
array( 'addPgIndex', 'iwlinks', 'iwl_prefix_from_title', '(iwl_prefix, iwl_from, iwl_title)' ),
array( 'addPgIndex', 'iwlinks', 'iwl_prefix_title_from', '(iwl_prefix, iwl_title, iwl_from)' ),
array( 'addPgIndex', 'job', 'job_timestamp_idx', '(job_timestamp)' ),
array( 'addPgIndex', 'job', 'job_sha1', '(job_sha1)' ),
@ -251,6 +252,12 @@ class PostgresUpdater extends DatabaseUpdater {
array( 'cl_from', 'int4_ops', 'btree', 0 ),
),
'CREATE INDEX cl_sortkey ON "categorylinks" USING "btree" ("cl_to", "cl_sortkey", "cl_from")' ),
array( 'checkIndex', 'iwl_prefix_title_from', array(
array('iwl_prefix', 'text_ops', 'btree', 0),
array('iwl_title', 'text_ops', 'btree', 0),
array('iwl_from', 'int4_ops', 'btree', 0),
),
'CREATE INDEX iwl_prefix_title_from ON "iwlinks" USING "btree" ("iwl_prefix", "iwl_title", "iwl_from")' ),
array( 'checkIndex', 'logging_times', array(
array( 'log_timestamp', 'timestamptz_ops', 'btree', 0 ),
),
@ -770,7 +777,7 @@ END;
protected function checkIwlPrefix() {
if ( $this->db->indexExists( 'iwlinks', 'iwl_prefix' ) ) {
$this->applyPatch( 'patch-rename-iwl_prefix.sql', false, "Replacing index 'iwl_prefix' with 'iwl_prefix_from_title'" );
$this->applyPatch( 'patch-rename-iwl_prefix.sql', false, "Replacing index 'iwl_prefix' with 'iwl_prefix_title_from'" );
}
}

View file

@ -62,7 +62,6 @@ class SqliteUpdater extends DatabaseUpdater {
array( 'addField', 'updatelog', 'ul_value', 'patch-ul_value.sql' ),
array( 'addField', 'interwiki', 'iw_api', 'patch-iw_api_and_wikiid.sql' ),
array( 'dropIndex', 'iwlinks', 'iwl_prefix', 'patch-kill-iwl_prefix.sql' ),
array( 'dropIndex', 'iwlinks', 'iwl_prefix_from_title', 'patch-kill-iwl_pft.sql' ),
array( 'addField', 'categorylinks', 'cl_collation', 'patch-categorylinks-better-collation.sql' ),
array( 'doCollationUpdate' ),
array( 'addTable', 'msg_resource', 'patch-msg_resource.sql' ),
@ -110,6 +109,7 @@ class SqliteUpdater extends DatabaseUpdater {
array( 'modifyField', 'user_former_groups', 'ufg_group', 'patch-ufg_group-length-increase-255.sql' ),
array( 'addIndex', 'page_props', 'pp_propname_page', 'patch-page_props-propname-page-index.sql' ),
array( 'addIndex', 'image', 'img_media_mime', 'patch-img_media_mime-index.sql' ),
array( 'addIndex', 'iwlinks', 'iwl_prefix_from_title', 'patch-iwlinks-from-title-index.sql' ),
);
}

View file

@ -0,0 +1,5 @@
--
-- Makes the iwl_prefix_title_from index for the iwlinks table non-unique
--
DROP INDEX /*i*/iwl_prefix_title_from ON /*_*/iwlinks;
CREATE INDEX /*i*/iwl_prefix_title_from ON /*_*/iwlinks (iwl_prefix, iwl_title, iwl_from);

View file

@ -0,0 +1,4 @@
--
-- Recreates the iwl_prefix_from_title index for the iwlinks table
--
CREATE INDEX /*i*/iwl_prefix_from_title ON /*_*/iwlinks (iwl_prefix, iwl_from, iwl_title);

View file

@ -1,7 +0,0 @@
--
-- Kill the old iwl_prefix_from_title index, which may be present on some
-- installs if they ran update.php between it being added and being renamed
--
DROP INDEX /*i*/iwl_prefix_from_title ON /*_*/iwlinks;

View file

@ -1,7 +0,0 @@
--
-- Kill the old iwl_prefix_from_title index, which may be present on some
-- installs if they ran update.php between it being added and being renamed
--
DROP INDEX iwl_prefix_from_title;

View file

@ -1,2 +1,2 @@
DROP INDEX iwl_prefix;
CREATE UNIQUE INDEX iwl_prefix_title_from ON iwlinks (iwl_prefix, iwl_from, iwl_title);
CREATE UNIQUE INDEX iwl_prefix_title_from ON iwlinks (iwl_prefix, iwl_title, iwl_from);

View file

@ -679,6 +679,7 @@ CREATE TABLE iwlinks (
);
CREATE UNIQUE INDEX iwl_from ON iwlinks (iwl_from, iwl_prefix, iwl_title);
CREATE UNIQUE INDEX iwl_prefix_title_from ON iwlinks (iwl_prefix, iwl_title, iwl_from);
CREATE UNIQUE INDEX iwl_prefix_from_title ON iwlinks (iwl_prefix, iwl_from, iwl_title);
CREATE TABLE msg_resource (
mr_resource TEXT NOT NULL,

View file

@ -1,7 +0,0 @@
--
-- Kill the old iwl_prefix_from_title index, which may be present on some
-- installs if they ran update.php between it being added and being renamed
--
DROP INDEX IF EXISTS /*i*/iwl_prefix;

View file

@ -2,4 +2,4 @@
-- Recreates the iwl_prefix for the iwlinks table
--
DROP INDEX IF EXISTS /*i*/iwl_prefix;
CREATE INDEX IF NOT EXISTS /*i*/iwl_prefix_from_title ON /*_*/iwlinks (iwl_prefix, iwl_from, iwl_title);
CREATE INDEX IF NOT EXISTS /*i*/iwl_prefix_title_from ON /*_*/iwlinks (iwl_prefix, iwl_title, iwl_from);

View file

@ -674,7 +674,8 @@ CREATE TABLE /*_*/iwlinks (
) /*$wgDBTableOptions*/;
CREATE UNIQUE INDEX /*i*/iwl_from ON /*_*/iwlinks (iwl_from, iwl_prefix, iwl_title);
CREATE UNIQUE INDEX /*i*/iwl_prefix_title_from ON /*_*/iwlinks (iwl_prefix, iwl_title, iwl_from);
CREATE INDEX /*i*/iwl_prefix_title_from ON /*_*/iwlinks (iwl_prefix, iwl_title, iwl_from);
CREATE INDEX /*i*/iwl_prefix_from_title ON /*_*/iwlinks (iwl_prefix, iwl_from, iwl_title);
--