From 3b9e6bec3e8d3a8efbed9806cc9de685c14e48ab Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 1 Mar 2018 20:41:32 -0800
Subject: [PATCH] rdbms: add missing hint check
 DatabaseMysqlBase::isInsertSelectSafe

This was lost when a bunch of other logic was split off in 671368a59e3

Change-Id: I3d3f744f8fce007ecf88cbd2c9f99918b06f0573
---
 .../libs/rdbms/database/DatabaseMysqlBase.php |  7 +-
 .../rdbms/database/DatabaseMysqlBaseTest.php  | 94 +++++++++++++++++++
 2 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 1e845e8d86..8a4c4bf386 100644
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -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,
diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
index 5fcca1a410..14c705734b 100644
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
@@ -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
+			],
+
+		];
+	}
 }
-- 
2.20.1