Passing empty array to wpdb::update() generates invalid SQL

[Viper007Bond]

I wanted to update all rows in a database table, so I did this:

$wpdb->update(
	'custom_table',
	array(
		'column' => 'value',
	),
	array()
);

However this generates a query like this:

UPDATE `custom_table` SET `column` = 'value' WHERE

So we either need some validation and bail if it's empty or better, we should drop the WHERE, allowing queries like mine.

[nacin]

Seems dangerous to allow an unbounded UPDATE with ease. Perhaps we could allow for something like exactly array( '1' => '1' ) (as in, WHERE 1=1) as an under-the-hood, explicit way of doing an unbounded UPDATE.

[Viper007Bond]

Replying to nacin:

Seems dangerous to allow an unbounded UPDATE with ease. Perhaps we could allow for something like exactly array( '1' => '1' ) (as in, WHERE 1=1) as an under-the-hood, explicit way of doing an unbounded UPDATE.

That could work, and infact that's what I tried before I realized that the first 1 would be treated as a column name.

[UmeshSingla]

Replying to Viper007Bond:

Replying to nacin:

Seems dangerous to allow an unbounded UPDATE with ease. Perhaps we could allow for something like exactly array( '1' => '1' ) (as in, WHERE 1=1) as an under-the-hood, explicit way of doing an unbounded UPDATE.

That could work, and infact that's what I tried before I realized that the first 1 would be treated as a column name.

There is a workaround for 1 being treated as column name, but a query something like that could screw the whole database like I ended up corrupting my whole user meta, if no precautions are taken.

[UmeshSingla]

Allow to update all rows in a table

[UmeshSingla]

Allow to update all rows in a table

[wonderboymusic]

In 28814:

In $wpdb->update(), prevent explosions when $where is empty.

Adds unit tests.

Props UmeshSingla, wonderboymusic.
Fixes #26106

[UmeshSingla]

@wonderboymusic, We are keeping the update query unbounded, I mean you've removed the 1=1 part from patch, is it intentional?

[nacin]

The 1=1 part doesn't matter as to whether it is unbounded. That was just a suggestion as to how you could make it "explicit" in that you wanted an unbounded query.

To be honest, I'm still very scared about this. One bad variable and suddenly you've updated all rows. I don't love this as an API change and would like to see if anyone else agrees for a possible 4.0 revert.

Two of the longest downtime events of WordPress.com in the last 4-5 years were, to my recollection, as follows:

• A bad variable name/assumption that overwrote entire options tables.
• An unbounded query that overwrote the entire domain mapping table.

This fix makes it trivial to combine both of these events into one. Let's not, please.

[nacin]

In this case, the update() method still rejects any $where that is not an array. So this requires an explicit empty array() to be passed. Is that really enough to prevent issues like this?

$where = array();
if ( something ) {
   $where[] = ...;
} elseif ( something else ) {
   $where[] = ...;
}

$wpdb->update( $table, $data, $where ); // oops, $where = array()

Maybe I'm just assuming too little of developers, but it's very possible that at least one plugin is making this mistake right now and we're going from a quiet SQL error to wiping the entire table with the 4.0 update.

I'd probably rather see separate update_all() and delete_all() methods or something.

[helen]

Revert sounds good to me. Empty array() is definitely not explicitly asking for an unbounded query.

[pento]

+1 for revert. I agree with the original premise of the ticket (that there should be a way to do unbounded updates), but this change makes it too easy for someone to make a disastrous mistake.

[nacin]

In 29664:

DB: Revert [28814] and require a WHERE for wpdb::update().

see #26106.

[nacin]

I left the unit tests here to enforce the design decision. If anyone tries to change this five years from now, the tests will fail, and they'll be led here.

[markjaquith]

100% agree with revert. Yikes. That code scenario Nacin outlined is entirely plausible.

[Viper007Bond]

As the reporter, I agree with the revert as well. pento sums it up nicely -- should be possible but not easy.

[SergeyBiryukov]

$wpdb->show_errors(false) does not work as expected in [29664]:

.............................................................   61 / 2504 (  2%)
...........................SS...SSSSSS.............S...SS....  122 / 2504 (  4%)
........SS................S...........SS...SSSSSS............  183 / 2504 (  7%)
.S...SS............SS..........................S.............  244 / 2504 (  9%)
.............................................................  305 / 2504 ( 12%)
.WordPress database error You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 for query UPDATE `wptests_posts` SET `post_name` = 'burrito' WHERE  made by PHPUnit_TextUI_Command::main, PHPUnit_TextUI_Command->run, PHPUnit_TextUI_TestRunner->doRun, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->runTest, PHPUnit_Framework_TestCase->run, PHPUnit_Framework_TestResult->run, PHPUnit_Framework_TestCase->runBare, PHPUnit_Framework_TestCase->runTest, ReflectionMethod->invokeArgs, Tests_DB->test_empty_where_on_update

26106.3.diff​ fixes that.

[SergeyBiryukov]

In 29701:

Properly suppress errors in test_empty_where_on_update(), which expects an empty WHERE clause.

see #26106.