Fix insertSelect() with IGNORE in PostgreSQL
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 20 Sep 2017 16:55:55 +0000 (12:55 -0400)
committerAnomie <bjorsch@wikimedia.org>
Tue, 26 Sep 2017 01:32:27 +0000 (01:32 +0000)
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
includes/libs/rdbms/database/DatabasePostgres.php

index e7417eb..7a97dcb 100644 (file)
@@ -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.
index 672b345..5719a1f 100644 (file)
@@ -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' ) {