(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 fc77b4e..6f03bbe 100644 (file)
--- 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 ac65d2d..e6b02d7 100644 (file)
--- 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 d8fa64e..6b74653 100644 (file)
--- 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 17285c5..0a319ee 100644 (file)
--- 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 11e3445..54e115b 100644 (file)
--- 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 (file)
index 0000000..bff63c7
--- /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 (file)
index 0000000..8b73f9e
--- /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 (file)
index 96e1435..0000000
--- 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 (file)
index 4419d9e..0000000
--- 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 a4bdb6a..0eb792e 100644 (file)
--- 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 9cbabfd..5ce1a17 100644 (file)
--- 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 (file)
index 8fc4b5c..0000000
--- 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 fd4c9ec..6d5b1bf 100644 (file)
--- 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 72b4eb6..fb145d0 100644 (file)
--- 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);
 
 
 --