From: Brad Jorsch Date: Fri, 11 Jan 2013 00:22:03 +0000 (-0500) Subject: (bug 36400) API: Fix sorting for iwlinks, langlinks X-Git-Tag: 1.31.0-rc.0~20115^2 X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/comptes/ajouter.php?a=commitdiff_plain;h=c013ec02b9219941b29030ae6ff9f7052df646ef;p=lhc%2Fweb%2Fwiklou.git (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 --- diff --git a/includes/api/ApiQueryIWLinks.php b/includes/api/ApiQueryIWLinks.php index fc77b4e6c4..6f03bbee45 100644 --- a/includes/api/ApiQueryIWLinks.php +++ b/includes/api/ApiQueryIWLinks.php @@ -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 )); } } diff --git a/includes/api/ApiQueryLangLinks.php b/includes/api/ApiQueryLangLinks.php index ac65d2d2e5..e6b02d79d9 100644 --- a/includes/api/ApiQueryLangLinks.php +++ b/includes/api/ApiQueryLangLinks.php @@ -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 ) { diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php index d8fa64e3a1..6b74653a6e 100644 --- a/includes/installer/MysqlUpdater.php +++ b/includes/installer/MysqlUpdater.php @@ -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" ); + } } diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index 17285c543a..0a319eeab5 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -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'" ); } } diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php index 11e3445518..54e115bd9f 100644 --- a/includes/installer/SqliteUpdater.php +++ b/includes/installer/SqliteUpdater.php @@ -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' ), ); } diff --git a/maintenance/archives/patch-iwl_prefix_title_from-non-unique.sql b/maintenance/archives/patch-iwl_prefix_title_from-non-unique.sql new file mode 100644 index 0000000000..bff63c745a --- /dev/null +++ b/maintenance/archives/patch-iwl_prefix_title_from-non-unique.sql @@ -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); diff --git a/maintenance/archives/patch-iwlinks-from-title-index.sql b/maintenance/archives/patch-iwlinks-from-title-index.sql new file mode 100644 index 0000000000..8b73f9e343 --- /dev/null +++ b/maintenance/archives/patch-iwlinks-from-title-index.sql @@ -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); diff --git a/maintenance/archives/patch-kill-iwl_pft.sql b/maintenance/archives/patch-kill-iwl_pft.sql deleted file mode 100644 index 96e1435693..0000000000 --- a/maintenance/archives/patch-kill-iwl_pft.sql +++ /dev/null @@ -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; - diff --git a/maintenance/postgres/archives/patch-kill-iwl_pft.sql b/maintenance/postgres/archives/patch-kill-iwl_pft.sql deleted file mode 100644 index 4419d9e9a8..0000000000 --- a/maintenance/postgres/archives/patch-kill-iwl_pft.sql +++ /dev/null @@ -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; - diff --git a/maintenance/postgres/archives/patch-rename-iwl_prefix.sql b/maintenance/postgres/archives/patch-rename-iwl_prefix.sql index a4bdb6a913..0eb792eae2 100644 --- a/maintenance/postgres/archives/patch-rename-iwl_prefix.sql +++ b/maintenance/postgres/archives/patch-rename-iwl_prefix.sql @@ -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); diff --git a/maintenance/postgres/tables.sql b/maintenance/postgres/tables.sql index 9cbabfdf23..5ce1a172d3 100644 --- a/maintenance/postgres/tables.sql +++ b/maintenance/postgres/tables.sql @@ -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, diff --git a/maintenance/sqlite/archives/patch-kill-iwl_pft.sql b/maintenance/sqlite/archives/patch-kill-iwl_pft.sql deleted file mode 100644 index 8fc4b5cd67..0000000000 --- a/maintenance/sqlite/archives/patch-kill-iwl_pft.sql +++ /dev/null @@ -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; - diff --git a/maintenance/sqlite/archives/patch-rename-iwl_prefix.sql b/maintenance/sqlite/archives/patch-rename-iwl_prefix.sql index fd4c9ec7e0..6d5b1bfad3 100644 --- a/maintenance/sqlite/archives/patch-rename-iwl_prefix.sql +++ b/maintenance/sqlite/archives/patch-rename-iwl_prefix.sql @@ -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); diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 72b4eb6cfa..fb145d0829 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -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); --