Make DatabaseMysqlBase::insertSelect() safer to use

Certain server configurations, including the current MariaDB defaults,
make INSERT SELECT unsafe for replication in MySQL/MariaDB. When the
server configuration is not known to be safe, force the use of the
non-native implementation. Note this only has effect in CLI mode, as
non-CLI mode already forces the non-native implementation since
I2dba6024.

Also, native INSERT SELECT won't be safe with any statement-based
replication method if the order of rows in the SELECT is not
deterministic. Add a warning to the method's documentation pointing this
out.

Change-Id: I9173f6559809bd01830bd0a9f443c7269cc58ce2

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 305a056..82a78ca 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -63,6 +63,8 @@ abstract class DatabaseMysqlBase extends Database {
 
        /** @var string|null */
        private $serverVersion = null;
+       /** @var bool|null */
+       private $insertSelectIsSafe = null;
 
        /**
         * Additional $params include:
@@ -75,6 +77,7 @@ abstract class DatabaseMysqlBase extends Database {
         *       ID of this server's master will be used. Set the "conds" field to
         *       override the query conditions, e.g. ['shard' => 's1'].
         *   - useGTIDs : use GTID methods like MASTER_GTID_WAIT() when possible.
+        *   - insertSelectIsSafe : force that native INSERT SELECT is or is not safe [default: null]
         *   - sslKeyPath : path to key file [default: null]
         *   - sslCertPath : path to certificate file [default: null]
         *   - sslCAFile: path to a single certificate authority PEM file [default: null]
@@ -98,6 +101,8 @@ abstract class DatabaseMysqlBase extends Database {
                }
                $this->sqlMode = isset( $params['sqlMode'] ) ? $params['sqlMode'] : '';
                $this->utf8Mode = !empty( $params['utf8Mode'] );
+               $this->insertSelectIsSafe = isset( $params['insertSelectIsSafe'] )
+                       ? (bool)$params['insertSelectIsSafe'] : null;
 
                parent::__construct( $params );
        }
@@ -501,6 +506,56 @@ abstract class DatabaseMysqlBase extends Database {
                return $this->nativeReplace( $table, $rows, $fname );
        }
 
+       protected function nativeInsertSelect(
+               $destTable, $srcTable, $varMap, $conds,
+               $fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
+       ) {
+               if ( $this->insertSelectIsSafe === null ) {
+                       // In MySQL, an INSERT SELECT is only replication safe with row-based
+                       // replication or if innodb_autoinc_lock_mode is 0. When those
+                       // conditions aren't met, use non-native mode.
+                       // While we could try to determine if the insert is safe anyway by
+                       // checking if the target table has an auto-increment column that
+                       // isn't set in $varMap, that seems unlikely to be worth the extra
+                       // complexity.
+                       $row = $this->selectRow(
+                               false,
+                               [
+                                       'innodb_autoinc_lock_mode' => '@@innodb_autoinc_lock_mode',
+                                       'binlog_format' => '@@binlog_format',
+                               ],
+                               [],
+                               __METHOD__
+                       );
+                       $this->insertSelectIsSafe = $row->binlog_format === 'ROW' ||
+                               (int)$row->innodb_autoinc_lock_mode === 0;
+               }
+
+               if ( !$this->insertSelectIsSafe ) {
+                       return $this->nonNativeInsertSelect(
+                               $destTable,
+                               $srcTable,
+                               $varMap,
+                               $conds,
+                               $fname,
+                               $insertOptions,
+                               $selectOptions,
+                               $selectJoinConds
+                       );
+               } else {
+                       return parent::nativeInsertSelect(
+                               $destTable,
+                               $srcTable,
+                               $varMap,
+                               $conds,
+                               $fname,
+                               $insertOptions,
+                               $selectOptions,
+                               $selectJoinConds
+                       );
+               }
+       }
+
        /**
         * Estimate rows in dataset
         * Returns estimated count, based on EXPLAIN output

diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index 85b3481..0964dd5 100644 (file)
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -1255,6 +1255,11 @@ interface IDatabase {
         * INSERT SELECT wrapper. Takes data from a SELECT query and inserts it
         * into another table.
         *
+        * @warning If the insert will use an auto-increment or sequence to
+        *  determine the value of a column, this may break replication on
+        *  databases using statement-based replication if the SELECT is not
+        *  deterministically ordered.
+        *
         * @param string $destTable The table name to insert into
         * @param string|array $srcTable May be either a table name, or an array of table names
         *    to include in a join.