DB: Add join conditions to selectField, selectFieldValues, and insertSelect

selectField() and selectFieldValues() are trivial, they just need to
pass it through to select(). In fact, selectFieldValues() was already
doing it, just no one ever updated IDatabase.

insertSelect() is a little more work. nativeInsertSelect() was
originally written as largely a copy-paste of select() and has since
gotten well out of sync. Now that we have selectSQLText(), we should be
able to just use that. DatabasePostgres's implementation can wrap the
parent implementation instead of being another copy-paste, but
DatabaseOracle seems to still need to be special.

Change-Id: I0e6a9e6daa510639d3212641606047a5db96c500

diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php
index b728786..556fe75 100644 (file)
--- a/includes/db/DatabaseOracle.php
+++ b/includes/db/DatabaseOracle.php
@@ -558,19 +558,9 @@ class DatabaseOracle extends Database {
        }
 
        function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
-               $insertOptions = [], $selectOptions = []
+               $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
                $destTable = $this->tableName( $destTable );
-               if ( !is_array( $selectOptions ) ) {
-                       $selectOptions = [ $selectOptions ];
-               }
-               list( $startOpts, $useIndex, $tailOpts, $ignoreIndex ) =
-                       $this->makeSelectOptions( $selectOptions );
-               if ( is_array( $srcTable ) ) {
-                       $srcTable = implode( ',', array_map( [ $this, 'tableName' ], $srcTable ) );
-               } else {
-                       $srcTable = $this->tableName( $srcTable );
-               }
 
                $sequenceData = $this->getSequenceData( $destTable );
                if ( $sequenceData !== false &&
@@ -585,13 +575,16 @@ class DatabaseOracle extends Database {
                        $val = $val . ' field' . ( $i++ );
                }
 
-               $sql = "INSERT INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ')' .
-                       " SELECT $startOpts " . implode( ',', $varMap ) .
-                       " FROM $srcTable $useIndex $ignoreIndex ";
-               if ( $conds != '*' ) {
-                       $sql .= ' WHERE ' . $this->makeList( $conds, LIST_AND );
-               }
-               $sql .= " $tailOpts";
+               $selectSql = $this->selectSQLText(
+                       $srcTable,
+                       array_values( $varMap ),
+                       $conds,
+                       $fname,
+                       $selectOptions,
+                       $selectJoinConds
+               );
+
+               $sql = "INSERT INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ') ' . $selectSql;
 
                if ( in_array( 'IGNORE', $insertOptions ) ) {
                        $this->ignoreDupValOnIndex = true;

diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php
index 5b59d2a..fb4122d 100644 (file)
--- a/includes/libs/rdbms/database/DBConnRef.php
+++ b/includes/libs/rdbms/database/DBConnRef.php
@@ -247,13 +247,13 @@ class DBConnRef implements IDatabase {
        }
 
        public function selectField(
-               $table, $var, $cond = '', $fname = __METHOD__, $options = []
+               $table, $var, $cond = '', $fname = __METHOD__, $options = [], $join_conds = []
        ) {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function selectFieldValues(
-               $table, $var, $cond = '', $fname = __METHOD__, $options = []
+               $table, $var, $cond = '', $fname = __METHOD__, $options = [], $join_conds = []
        ) {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
@@ -411,7 +411,7 @@ class DBConnRef implements IDatabase {
 
        public function insertSelect(
                $destTable, $srcTable, $varMap, $conds,
-               $fname = __METHOD__, $insertOptions = [], $selectOptions = []
+               $fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
                return $this->__call( __FUNCTION__, func_get_args() );
        }

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 8bea8cc..9e91592 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1150,7 +1150,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function selectField(
-               $table, $var, $cond = '', $fname = __METHOD__, $options = []
+               $table, $var, $cond = '', $fname = __METHOD__, $options = [], $join_conds = []
        ) {
                if ( $var === '*' ) { // sanity
                        throw new DBUnexpectedError( $this, "Cannot use a * field: got '$var'" );
@@ -1162,7 +1162,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                $options['LIMIT'] = 1;
 
-               $res = $this->select( $table, $var, $cond, $fname, $options );
+               $res = $this->select( $table, $var, $cond, $fname, $options, $join_conds );
                if ( $res === false || !$this->numRows( $res ) ) {
                        return false;
                }
@@ -2356,7 +2356,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
        public function insertSelect(
                $destTable, $srcTable, $varMap, $conds,
-               $fname = __METHOD__, $insertOptions = [], $selectOptions = []
+               $fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
                if ( $this->cliMode ) {
                        // For massive migrations with downtime, we don't want to select everything
@@ -2368,7 +2368,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                $conds,
                                $fname,
                                $insertOptions,
-                               $selectOptions
+                               $selectOptions,
+                               $selectJoinConds
                        );
                }
 
@@ -2380,7 +2381,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $fields[] = $this->fieldNameWithAlias( $sourceColumnOrSql, $dstColumn );
                }
                $selectOptions[] = 'FOR UPDATE';
-               $res = $this->select( $srcTable, implode( ',', $fields ), $conds, $fname, $selectOptions );
+               $res = $this->select(
+                       $srcTable, implode( ',', $fields ), $conds, $fname, $selectOptions, $selectJoinConds
+               );
                if ( !$res ) {
                        return false;
                }
@@ -2401,7 +2404,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         */
        protected function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds,
                $fname = __METHOD__,
-               $insertOptions = [], $selectOptions = []
+               $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
                $destTable = $this->tableName( $destTable );
 
@@ -2411,32 +2414,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                $insertOptions = $this->makeInsertOptions( $insertOptions );
 
-               if ( !is_array( $selectOptions ) ) {
-                       $selectOptions = [ $selectOptions ];
-               }
-
-               list( $startOpts, $useIndex, $tailOpts, $ignoreIndex ) = $this->makeSelectOptions(
-                       $selectOptions );
-
-               if ( is_array( $srcTable ) ) {
-                       $srcTable = implode( ',', array_map( [ $this, 'tableName' ], $srcTable ) );
-               } else {
-                       $srcTable = $this->tableName( $srcTable );
-               }
+               $selectSql = $this->selectSQLText(
+                       $srcTable,
+                       array_values( $varMap ),
+                       $conds,
+                       $fname,
+                       $selectOptions,
+                       $selectJoinConds
+               );
 
                $sql = "INSERT $insertOptions" .
-                       " INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ')' .
-                       " SELECT $startOpts " . implode( ',', $varMap ) .
-                       " FROM $srcTable $useIndex $ignoreIndex ";
-
-               if ( $conds != '*' ) {
-                       if ( is_array( $conds ) ) {
-                               $conds = $this->makeList( $conds, self::LIST_AND );
-                       }
-                       $sql .= " WHERE $conds";
-               }
-
-               $sql .= " $tailOpts";
+                       " INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ') ' .
+                       $selectSql;
 
                return $this->query( $sql, $fname );
        }

diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php
index 782727a..8f3cab8 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMssql.php
+++ b/includes/libs/rdbms/database/DatabaseMssql.php
@@ -717,11 +717,12 @@ class DatabaseMssql extends Database {
         * @param string $fname
         * @param array $insertOptions
         * @param array $selectOptions
+        * @param array $selectJoinConds
         * @return null|ResultWrapper
         * @throws Exception
         */
        public function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
-               $insertOptions = [], $selectOptions = []
+               $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
                $this->mScrollableCursor = false;
                try {
@@ -732,7 +733,8 @@ class DatabaseMssql extends Database {
                                $conds,
                                $fname,
                                $insertOptions,
-                               $selectOptions
+                               $selectOptions,
+                               $selectJoinConds
                        );
                } catch ( Exception $e ) {
                        $this->mScrollableCursor = true;

diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index 2fe275b..fe5cfa1 100644 (file)
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -681,14 +681,13 @@ __INDEXATTR__;
         * @param string $fname
         * @param array $insertOptions
         * @param array $selectOptions
+        * @param array $selectJoinConds
         * @return bool
         */
        public function nativeInsertSelect(
                $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
-               $insertOptions = [], $selectOptions = []
+               $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
-               $destTable = $this->tableName( $destTable );
-
                if ( !is_array( $insertOptions ) ) {
                        $insertOptions = [ $insertOptions ];
                }
@@ -705,28 +704,9 @@ __INDEXATTR__;
                        $savepoint->savepoint();
                }
 
-               if ( !is_array( $selectOptions ) ) {
-                       $selectOptions = [ $selectOptions ];
-               }
-               list( $startOpts, $useIndex, $tailOpts, $ignoreIndex ) =
-                       $this->makeSelectOptions( $selectOptions );
-               if ( is_array( $srcTable ) ) {
-                       $srcTable = implode( ',', array_map( [ $this, 'tableName' ], $srcTable ) );
-               } else {
-                       $srcTable = $this->tableName( $srcTable );
-               }
-
-               $sql = "INSERT INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ')' .
-                       " SELECT $startOpts " . implode( ',', $varMap ) .
-                       " FROM $srcTable $useIndex $ignoreIndex ";
-
-               if ( $conds != '*' ) {
-                       $sql .= ' WHERE ' . $this->makeList( $conds, LIST_AND );
-               }
-
-               $sql .= " $tailOpts";
+               $res = parent::nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname,
+                       $insertOptions, $selectOptions, $selectJoinConds );
 
-               $res = (bool)$this->query( $sql, $fname, $savepoint );
                if ( $savepoint ) {
                        $bar = pg_result_error( $this->mLastResult );
                        if ( $bar != false ) {

diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index 617982c..7c6413c 100644 (file)
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -568,11 +568,12 @@ interface IDatabase {
         * @param string|array $cond The condition array. See IDatabase::select() for details.
         * @param string $fname The function name of the caller.
         * @param string|array $options The query options. See IDatabase::select() for details.
+        * @param string|array $join_conds The query join conditions. See IDatabase::select() for details.
         *
         * @return bool|mixed The value from the field, or false on failure.
         */
        public function selectField(
-               $table, $var, $cond = '', $fname = __METHOD__, $options = []
+               $table, $var, $cond = '', $fname = __METHOD__, $options = [], $join_conds = []
        );
 
        /**
@@ -589,12 +590,13 @@ interface IDatabase {
         * @param string|array $cond The condition array. See IDatabase::select() for details.
         * @param string $fname The function name of the caller.
         * @param string|array $options The query options. See IDatabase::select() for details.
+        * @param string|array $join_conds The query join conditions. See IDatabase::select() for details.
         *
         * @return bool|array The values from the field, or false on failure
         * @since 1.25
         */
        public function selectFieldValues(
-               $table, $var, $cond = '', $fname = __METHOD__, $options = []
+               $table, $var, $cond = '', $fname = __METHOD__, $options = [], $join_conds = []
        );
 
        /**
@@ -1247,12 +1249,14 @@ interface IDatabase {
         *    IDatabase::insert() for details.
         * @param array $selectOptions Options for the SELECT part of the query, see
         *    IDatabase::select() for details.
+        * @param array $selectJoinConds Join conditions for the SELECT part of the query, see
+        *    IDatabase::select() for details.
         *
         * @return IResultWrapper
         */
        public function insertSelect( $destTable, $srcTable, $varMap, $conds,
                $fname = __METHOD__,
-               $insertOptions = [], $selectOptions = []
+               $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        );
 
        /**

diff --git a/tests/phpunit/includes/db/DatabaseSQLTest.php b/tests/phpunit/includes/db/DatabaseSQLTest.php
index b6088ff..206655c 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/db/DatabaseSQLTest.php
@@ -17,13 +17,13 @@ class DatabaseSQLTest extends MediaWikiTestCase {
 
        protected function assertLastSql( $sqlText ) {
                $this->assertEquals(
-                       $this->database->getLastSqls(),
-                       $sqlText
+                       $sqlText,
+                       $this->database->getLastSqls()
                );
        }
 
        protected function assertLastSqlDb( $sqlText, $db ) {
-               $this->assertEquals( $db->getLastSqls(), $sqlText );
+               $this->assertEquals( $sqlText, $db->getLastSqls() );
        }
 
        /**
@@ -365,7 +365,8 @@ class DatabaseSQLTest extends MediaWikiTestCase {
                        $sql['conds'],
                        __METHOD__,
                        isset( $sql['insertOptions'] ) ? $sql['insertOptions'] : [],
-                       isset( $sql['selectOptions'] ) ? $sql['selectOptions'] : []
+                       isset( $sql['selectOptions'] ) ? $sql['selectOptions'] : [],
+                       isset( $sql['selectJoinConds'] ) ? $sql['selectJoinConds'] : []
                );
                $this->assertLastSql( $sqlTextNative );
 
@@ -380,7 +381,8 @@ class DatabaseSQLTest extends MediaWikiTestCase {
                        $sql['conds'],
                        __METHOD__,
                        isset( $sql['insertOptions'] ) ? $sql['insertOptions'] : [],
-                       isset( $sql['selectOptions'] ) ? $sql['selectOptions'] : []
+                       isset( $sql['selectOptions'] ) ? $sql['selectOptions'] : [],
+                       isset( $sql['selectJoinConds'] ) ? $sql['selectJoinConds'] : []
                );
                $this->assertLastSqlDb( implode( '; ', [ $sqlSelect, $sqlInsert ] ), $dbWeb );
        }
@@ -397,7 +399,7 @@ class DatabaseSQLTest extends MediaWikiTestCase {
                                "INSERT INTO insert_table " .
                                        "(field_insert,field) " .
                                        "SELECT field_select,field2 " .
-                                       "FROM select_table",
+                                       "FROM select_table WHERE *",
                                "SELECT field_select AS field_insert,field2 AS field " .
                                "FROM select_table WHERE *   FOR UPDATE",
                                "INSERT INTO insert_table (field_insert,field) VALUES ('0','1')"
@@ -437,6 +439,28 @@ class DatabaseSQLTest extends MediaWikiTestCase {
                                "FROM select_table WHERE field = '2' ORDER BY field  FOR UPDATE",
                                "INSERT IGNORE INTO insert_table (field_insert,field) VALUES ('0','1')"
                        ],
+                       [
+                               [
+                                       'destTable' => 'insert_table',
+                                       'srcTable' => [ 'select_table1', 'select_table2' ],
+                                       'varMap' => [ 'field_insert' => 'field_select', 'field' => 'field2' ],
+                                       'conds' => [ 'field' => 2 ],
+                                       'selectOptions' => [ 'ORDER BY' => 'field', 'FORCE INDEX' => [ 'select_table1' => 'index1' ] ],
+                                       'selectJoinConds' => [
+                                               'select_table2' => [ 'LEFT JOIN', [ 'select_table1.foo = select_table2.bar' ] ],
+                                       ],
+                               ],
+                               "INSERT INTO insert_table " .
+                                       "(field_insert,field) " .
+                                       "SELECT field_select,field2 " .
+                                       "FROM select_table1 LEFT JOIN select_table2 ON ((select_table1.foo = select_table2.bar)) " .
+                                       "WHERE field = '2' " .
+                                       "ORDER BY field",
+                               "SELECT field_select AS field_insert,field2 AS field " .
+                               "FROM select_table1 LEFT JOIN select_table2 ON ((select_table1.foo = select_table2.bar)) " .
+                               "WHERE field = '2' ORDER BY field  FOR UPDATE",
+                               "INSERT INTO insert_table (field_insert,field) VALUES ('0','1')"
+                       ],
                ];
        }