add $wpdb->upsert function

[thomask]

If i read ​http://codex.wordpress.org/Class_Reference/wpdb right, we have only insert and update functions, but for many occations, if would be lovely, if we would have upsert function as well (using INSERT ... ON DUPLICATE KEY UPDATE ​http://dev.mysql.com/doc/refman/5.1/en/insert-on-duplicate.html).

So when i am inserting e.g. posts using database function, i do not have to check if post with that id exist (good for some migration / importing tools etc.)

[wonderboymusic]

This one would be tricky:

Inserts and Updates also take formatting as an argument. If you pass the primary key as an arbitrary value, the primary key and its formatting would have to be removed from the values AND formatting arrays, regardless of what order they were passed.

The resulting SQL needs to look like:

INSERT INTO wp_posts (post_title, post_date, ID, post_content, post_type) 
VALUES ('Title', '2012-10-07 07:00:00', 3, 'This is content', 'event') 
ON DUPLICATE KEY UPDATE 
post_title = 'Title',
post_date ='2012-10-07 07:00:00',  
post_content = 'This is content', 
post_type = 'event'

But that can be passed to $wpdb->insert like so:

$wpdb->insert( 
	'wp_posts' 
	array( 
		'post_title' 	=> 'Title', 
		'post_date' 	=> '2012-10-07 07:00:00', 
		'ID' 		=> 3, 
		'post_content' 	=> 'This is content', 
		'post_type' 	=> 'event' 
	),
	array( '%s', '%s', '%d', '%s', '%s' )
);

The insert method takes arbitrary arrays as arguments, but to remove the *specific* primary key for any passed table, it would have to be intercepted and altered just in time to create the proper update SQL.

I'm not sure wpdb should get this fancy. Semi-related: https://core.trac.wordpress.org/ticket/19019

[wonderboymusic]

I'm the only comment, and that was 20 months ago.

[donpark]

Proposed patch adds new method upsert to wp-db.php and modifies private method _insert_replace_helper, adding a new $type value UPSERT.

public function upsert( $table, $data, $format = null, $keys = null )

Optional parameter $keys is used to filter out keys from ON DUPLICATE KEY UPDATE part of the query like this:

$wpdb->upsert( 'table', array('id' => 1234, 'column' => 'foo', 'field' => 1337 ),
    array('%d', '%s', '%d' ), array( 'id' ) );

[donpark]

UPSERT Patch

[pento]

Thanks for the patch, @donpark!

MySQL's INSERT ... ON DUPLICATE KEY UPDATE is... kind of weird, particularly on tables with multiple unique columns. If the INSERT clashes with multiple rows, only the first row encountered will be updated. I think the behaviour of which row will be updated is undefined. It's useful in certain circumstances, but can be horrifyingly hard to debug when used incorrectly. I'm not convinced that it's wise to be encouraging the use of upserts with a CRUD function, though I'm interested in your thoughts on the matter.

For the patch itself. I think that, instead of $keys, we'd want to pass $update_columns - the array of columns to be used in the UPDATE part of the query. This matches the behaviour of the actual query (you specify the columns to update, not the columns from the INSERT part to not update), and it's probably easier to maintain against table schema updates - adding a new unique index to a table would involve updating every upsert() call for that query.

I don't think there'd be any reason to change the mysqli_affected_rows() call to recognise upserts. It already has well defined behaviour for when the INSERT vs. the UPDATE part of the query was run.

There are also a couple of places in Core where we use upserts, updating them so we're using the functionality in Core would be good.

Also, this will need many unit tests. :-)

[donpark]

Replying to pento:

MySQL's INSERT ... ON DUPLICATE KEY UPDATE is... kind of weird, ...
though I'm interested in your thoughts on the matter.

I wasn't aware of MySQL's UPSERT issues. I'll need do some research before I can share anything.

Also, this will need many unit tests. :-)

I agree. :-)

[donpark]

Urgh. Trac UX leaves a lot to be desired.

Undoing unintended change to Resolution and Status.

[donpark]

Details on UPSERT problem @pento mentioned can be found here:
​https://stackoverflow.com/questions/16377932/mysql-behavior-of-on-duplicate-key-update-for-multiple-unique-fields
Duplicate keys are matched with OR, not AND, which makes no sense to me.
Undefined behavior stems from undefined order of duplicate key matching rows.

In short, UPSERT on MySQL needs to be used with care. Frankly I think this may be too much to ask for from typical WP hackers, even with flashing warning message.

Changing status to maybelater again to be reconsidered when and if demand for upsert warrants reconsideration.

Thx for the quick response @pento.

[thomask]

I am not an expert in this area, but wouldn't be a solution to that problem be allowing upsert for single ID? This is anyway the most typical scenario. But i am fine if it would be maybelater and wait for some mysql change, as i have reported this 5 years ago and reaction was minimal

[donpark]

@pento, would @thomask's one-key-upsert help w/upsert needs in the core you mentioned?

It doesn't help me but I can assist if the need is there.

[pento]

I think we're stuck between a rock and a hard place, here.

On the one hand, MySQL's upsert behaviour is unpredictable under some circumstances, so making it easier to use seems unwise.

On the other hand, having WPDB's upsert behave differently to MySQL seems like a recipe for people running into edge cases that they didn't expect. There'd also need to be a tradeoff between doing a complete set of pre-flight checks on upsert queries (for example, running a SELECT query first, based on any columns being updated that are part of unique keys) while also maintaining compatibility with non-transactional DB engines, like MyISAM.

I don't really have a good answer here, unfortunately. I think the best method for now is to require upsert queries to be written manually - people who are aware of the query can use it, but we're not encouraging wider use of something that is often not entirely understood.