rdbms: add missing hint check DatabaseMysqlBase::isInsertSelectSafe
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 2 Mar 2018 04:41:32 +0000 (20:41 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 2 Mar 2018 05:03:54 +0000 (21:03 -0800)
This was lost when a bunch of other logic was split off in 671368a59e3

Change-Id: I3d3f744f8fce007ecf88cbd2c9f99918b06f0573

includes/libs/rdbms/database/DatabaseMysqlBase.php
tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php

index 1e845e8..8a4c4bf 100644 (file)
@@ -527,13 +527,16 @@ abstract class DatabaseMysqlBase extends Database {
                // 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.
-               return ( (int)$row->innodb_autoinc_lock_mode === 0 );
+               return (
+                       in_array( 'NO_AUTO_COLUMNS', $insertOptions ) ||
+                       (int)$row->innodb_autoinc_lock_mode === 0
+               );
        }
 
        /**
         * @return stdClass Process cached row
         */
-       private function getReplicationSafetyInfo() {
+       protected function getReplicationSafetyInfo() {
                if ( $this->replicationInfoRow === null ) {
                        $this->replicationInfoRow = $this->selectRow(
                                false,
index 5fcca1a..14c7057 100644 (file)
@@ -29,6 +29,7 @@ use Wikimedia\Rdbms\MySQLMasterPos;
 use Wikimedia\Rdbms\DatabaseMysqlBase;
 use Wikimedia\Rdbms\DatabaseMysqli;
 use Wikimedia\Rdbms\Database;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * Fake class around abstract class so we can call concrete methods.
@@ -510,4 +511,97 @@ class DatabaseMysqlBaseTest extends PHPUnit\Framework\TestCase {
 
                $this->assertEquals( $pos, $roundtripPos );
        }
+
+       /**
+        * @covers Wikimedia\Rdbms\DatabaseMysqlBase::isInsertSelectSafe
+        * @dataProvider provideInsertSelectCases
+        */
+       public function testInsertSelectIsSafe( $insertOpts, $selectOpts, $row, $safe ) {
+               $db = $this->getMockBuilder( DatabaseMysqli::class )
+                       ->disableOriginalConstructor()
+                       ->setMethods( [ 'getReplicationSafetyInfo' ] )
+                       ->getMock();
+               $db->method( 'getReplicationSafetyInfo' )->willReturn( (object)$row );
+               $dbw = TestingAccessWrapper::newFromObject( $db );
+
+               $this->assertEquals( $safe, $dbw->isInsertSelectSafe( $insertOpts, $selectOpts ) );
+       }
+
+       public function provideInsertSelectCases() {
+               return [
+                       [
+                               [],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => '2',
+                                       'binlog_format' => 'ROW',
+                               ],
+                               true
+                       ],
+                       [
+                               [],
+                               [ 'LIMIT' => 100 ],
+                               [
+                                       'innodb_autoinc_lock_mode' => '2',
+                                       'binlog_format' => 'ROW',
+                               ],
+                               true
+                       ],
+                       [
+                               [],
+                               [ 'LIMIT' => 100 ],
+                               [
+                                       'innodb_autoinc_lock_mode' => '0',
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               false
+                       ],
+                       [
+                               [],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => '2',
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               false
+                       ],
+                       [
+                               [ 'NO_AUTO_COLUMNS' ],
+                               [ 'LIMIT' => 100 ],
+                               [
+                                       'innodb_autoinc_lock_mode' => '0',
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               false
+                       ],
+                       [
+                               [],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => 0,
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               true
+                       ],
+                       [
+                               [ 'NO_AUTO_COLUMNS' ],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => 2,
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               true
+                       ],
+                       [
+                               [ 'NO_AUTO_COLUMNS' ],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => 0,
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               true
+                       ],
+
+               ];
+       }
 }