From d98a666d38ecb27cf3b100e3e3c056bf2e639d3c Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Wed, 20 Sep 2017 12:55:55 -0400
Subject: [PATCH] Fix insertSelect() with IGNORE in PostgreSQL

I0e6a9e6d overlooked the special handling PG needs (prior to 9.5 anyway)
to properly emulate MySQL's IGNORE option when delegating to the parent
implementation.

For now, then, don't use the native implementation in PG when IGNORE is
specified. Instead, fall back to the non-native implementation that does
a select() then an insert() where PG can handle the IGNORE properly.

In the future we might use the ON CONFLICT DO NOTHING clause added in PG
9.5 to be able to do native insertSelect() with IGNORE (and to better
handle multi-row insert() with IGNORE, and we could use the related ON
CONFLICT DO UPDATE to implement upsert()). All that is left for a future
patch.

Change-Id: I7987d59580543de03d5c6a5ed7fa3ce551ac12f3
---
 includes/libs/rdbms/database/Database.php     | 31 ++++++++++++++++
 .../libs/rdbms/database/DatabasePostgres.php  | 35 ++++---------------
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index e7417eb84a..7a97dcb702 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2407,6 +2407,37 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 			);
 		}
 
+		return $this->nonNativeInsertSelect(
+			$destTable,
+			$srcTable,
+			$varMap,
+			$conds,
+			$fname,
+			$insertOptions,
+			$selectOptions,
+			$selectJoinConds
+		);
+	}
+
+	/**
+	 * Implementation of insertSelect() based on select() and insert()
+	 *
+	 * @see IDatabase::insertSelect()
+	 * @since 1.30
+	 * @param string $destTable
+	 * @param string|array $srcTable
+	 * @param array $varMap
+	 * @param array $conds
+	 * @param string $fname
+	 * @param array $insertOptions
+	 * @param array $selectOptions
+	 * @param array $selectJoinConds
+	 * @return bool
+	 */
+	protected function nonNativeInsertSelect( $destTable, $srcTable, $varMap, $conds,
+		$fname = __METHOD__,
+		$insertOptions = [], $selectOptions = [], $selectJoinConds = []
+	) {
 		// For web requests, do a locking SELECT and then INSERT. This puts the SELECT burden
 		// on only the master (without needing row-based-replication). It also makes it easy to
 		// know how big the INSERT is going to be.
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index 672b3458ca..5719a1fcea 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -597,6 +597,7 @@ __INDEXATTR__;
 		}
 
 		// If IGNORE is set, we use savepoints to emulate mysql's behavior
+		// @todo If PostgreSQL 9.5+, we could use ON CONFLICT DO NOTHING instead
 		$savepoint = $olde = null;
 		$numrowsinserted = 0;
 		if ( in_array( 'IGNORE', $options ) ) {
@@ -710,39 +711,17 @@ __INDEXATTR__;
 		}
 
 		/*
-		 * If IGNORE is set, we use savepoints to emulate mysql's behavior
-		 * Ignore LOW PRIORITY option, since it is MySQL-specific
+		 * If IGNORE is set, use the non-native version.
+		 * @todo If PostgreSQL 9.5+, we could use ON CONFLICT DO NOTHING
 		 */
-		$savepoint = $olde = null;
-		$numrowsinserted = 0;
 		if ( in_array( 'IGNORE', $insertOptions ) ) {
-			$savepoint = new SavepointPostgres( $this, 'mw', $this->queryLogger );
-			$olde = error_reporting( 0 );
-			$savepoint->savepoint();
+			return $this->nonNativeInsertSelect(
+				$destTable, $srcTable, $varMap, $conds, $fname, $insertOptions, $selectOptions, $selectJoinConds
+			);
 		}
 
-		$res = parent::nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname,
+		return parent::nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname,
 			$insertOptions, $selectOptions, $selectJoinConds );
-
-		if ( $savepoint ) {
-			$bar = pg_result_error( $this->mLastResult );
-			if ( $bar != false ) {
-				$savepoint->rollback();
-			} else {
-				$savepoint->release();
-				$numrowsinserted++;
-			}
-			error_reporting( $olde );
-			$savepoint->commit();
-
-			// Set the affected row count for the whole operation
-			$this->mAffectedRows = $numrowsinserted;
-
-			// IGNORE always returns true
-			return true;
-		}
-
-		return $res;
 	}
 
 	public function tableName( $name, $format = 'quoted' ) {
-- 
2.20.1