r71609 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r71608‎ | r71609 | r71610 >
Date:01:03, 25 August 2010
Author:reedy
Status:reverted (Comments)
Tags:
Comment:
Create insertOnDupeUpdate, called by insert

Will be using soon
Modified paths:
  • /trunk/phase3/includes/db/Database.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/Database.php
@@ -1097,7 +1097,7 @@
10981098 }
10991099 return !$indexInfo[0]->Non_unique;
11001100 }
1101 -
 1101+
11021102 /**
11031103 * INSERT wrapper, inserts an array into a table
11041104 *
@@ -1115,6 +1115,28 @@
11161116 * @return bool
11171117 */
11181118 function insert( $table, $a, $fname = 'Database::insert', $options = array() ) {
 1119+ return this->insertOnDupeUpdate( $table, $a, $fname, $options );
 1120+ }
 1121+
 1122+ /**
 1123+ * INSERT ... ON DUPE UPDATE wrapper, inserts an array into a table, optionally updating if
 1124+ * duplicate primary key found
 1125+ *
 1126+ * $a may be a single associative array, or an array of these with numeric keys, for
 1127+ * multi-row insert.
 1128+ *
 1129+ * Usually aborts on failure
 1130+ * If errors are explicitly ignored, returns success
 1131+ *
 1132+ * @param $table String: table name (prefix auto-added)
 1133+ * @param $a Array: Array of rows to insert
 1134+ * @param $fname String: Calling function name (use __METHOD__) for logs/profiling
 1135+ * @param $options Mixed: Associative array of options
 1136+ * @param $onDupeUpdate Array: Associative array of fields to update on duplicate
 1137+ *
 1138+ * @return bool
 1139+ */
 1140+ function insertOnDupeUpdate( $table, $a, $fname = 'Database::insertOnDupeUpdate', $options = array(), $onDupeUpdate = array() ) {
11191141 # No rows to insert, easy just return now
11201142 if ( !count( $a ) ) {
11211143 return true;
@@ -1148,6 +1170,11 @@
11491171 } else {
11501172 $sql .= '(' . $this->makeList( $a ) . ')';
11511173 }
 1174+
 1175+ if ( count( $onDupeUpdate ) ) {
 1176+ $sql .= ' ON DUPLICATE KEY UPDATE ' . $this->makeList( $onDupeUpdate );
 1177+ }
 1178+
11521179 return (bool)$this->query( $sql, $fname );
11531180 }
11541181

Follow-up revisions

RevisionCommit summaryAuthorDate
r71617Followup r71609 add missing $reedy02:24, 25 August 2010
r71791Style fixes for r71609 and follow-ups...simetrical16:42, 27 August 2010

Comments

#Comment by 😂 (talk | contribs)   01:12, 25 August 2010

Suggest moving this to DatabaseMysql and making insert() abstract. All the child classes implement it anyway.

#Comment by Reedy (talk | contribs)   01:14, 25 August 2010

SQLLite calls parent::insert()...

#Comment by 😂 (talk | contribs)   01:19, 25 August 2010

So it does...

This is some ugly code. Lots of duplication. Needs some lovin' :(

#Comment by Reedy (talk | contribs)   01:29, 25 August 2010

Mmmm.

Some of the pre-implementations are very scary.

Roan has suggested not having the seperate method, and having it as an optional parameter...

#Comment by Nikerabbit (talk | contribs)   06:30, 25 August 2010

The existing options parameter is not suitable either?

#Comment by Reedy (talk | contribs)   15:21, 25 August 2010

http://dev.mysql.com/doc/refman/5.0/en/insert.html

http://dev.mysql.com/doc/refman/5.0/en/insert-on-duplicate.html

It's shown as the last part of the query, so I'm guessing, forcing it into where the options would put it - at the start, for [LOW_PRIORITY | DELAYED | HIGH_PRIORITY]...

#Comment by MaxSem (talk | contribs)   15:31, 25 August 2010

DatabaseBase shouldn't contain database-specific SQL, I've been fighting this for the last year. This syntax doesn't work for SQLite, PG, and I haven't even looked at other DBs' docs.

#Comment by Reedy (talk | contribs)   17:47, 25 August 2010

Moved in r71634, forgot to reference

Status & tagging log