wpdb insert & update with null values

[westi]

From: ​http://wordpress.org/support/topic/bug-fix-wpdb-insert-amp-update-with-null-values

"Some of you might have noticed but the wpdb's insert & update methods don't work well with null value (if you try to set null on a numeric field you'll end up with 0)"

[hakre]

I don't think that wpdb->prepare is designed to handle NULL values for numeric fields. Probably that's the cause.

[jane]

If someone wants to get this in for 3.1, please post a patch now for community review, as freeze is right around the corner.

[westi]

Patch looks reasonable.
Have you got some example code which relies on this feature to test with?

[nacin]

Code appears to act only on the value being === null. If someone is currently passing a value that may end up being null, but is actually passing this to %s or with the intention of it being handled by a zero-length string, then we'll get an off-by-one error. It's also difficult to follow -- the arrays for fields/values and formats should ideally be easier to match up from a code review perspective.

This needs to be explicit, i.e. array( 'field1' => '1', 'field2' => null, 'field3' => 3 ), array( '%s', null, '%d' ).

Both NULL and 'null' as a case-insensitive string should probably be accepted as a format.

[westi]

Enhancement moving to Future Release - we are focused on bug fixes for 3.1 now

[TechDagan]

Where do we stand with this? As far as I can tell, it wasn't added to 3.2 and we're now in the 3.3 beta.

[jbutkus]

What are the plans, for implementing fully-boiled NULL handling?

[nacin]

Seems like a patch is needed to add null support to prepare(), see #12819.

[miqrogroove]

Replying to nacin:

Both NULL and 'null' as a case-insensitive string should probably be accepted as a format.

Accepting 'null' as a string equivalent of NULL will break every possible usage of that phrase within WordPress. Are you prepared to deal with that?

[miqrogroove]

I would also like to remind everyone that NULL is often invalid in MySQL, e.g.

"To search for column values that are NULL, you cannot use an expr = NULL test ... because expr = NULL is never true for any expression" -- MySQL refman

[jbutkus]

Replying to miqrogroove:

I would also like to remind everyone that NULL is often invalid in MySQL, e.g.

"To search for column values that are NULL, you cannot use an expr = NULL test ... because expr = NULL is never true for any expression" -- MySQL refman

The NULL is not invalid.
It is just that there is special operation for NULL checking IS NULL, instead of = NULL.

That is just to be expected, as field value is not NULL. The NULL is just an indicator, that field does not have any value.

But I fully agree with comment 18 by miqrogroove, where 'null' (string) usage in favor of NULL (literary PHP NULL value) is questioned. It would raise much more questions and issues, than come of use to developers.

And if nacin was talking about format string purely (i.e. use 'null' instead of '%s') - I can hardly understand the purpose of this. If I would like to use NULL for field, I would just skip that field from SQL statement, instead of adding 'null' format mask.

[TechDagan]

Replying to jbutkus:

If I would like to use NULL for field, I would just skip that field from SQL statement, instead of adding 'null' format mask.

In the case of an INSERT, that would work fine. But a field cannot be updated to become NULL, only to contain an empty string or, in the case of an integer, 0. Boolean fields are completely unmanageable.

I can see why the original version(s) of wpdb did not bother to include null-parsing logic, but it's not really that complicated to do. I'll take a look at the code and, time willing, see about submitting a patch.

[jbutkus]

Replying to TechDagan:

In the case of an INSERT, that would work fine. But a field cannot be updated to become NULL, only to contain an empty string or, in the case of an integer, 0. Boolean fields are completely unmanageable.

I would go for one of:

• do not provide a field (in format, or values list) if you don't want it to be touched;
• provide field with it's indended format (be it %s, or %d, etc.) and a value of NULL, in case you want to INSERT/UPDATE field with NULL.

[jbutkus]

Please find my patch attached.

I have tested it to work with such cases:

Test1:

$wpdb->prepare( 'INSERT INTO foo ( val1, val2, val3 ) VALUES ( x\'%2$s\', %3$d, \'%1$s\' ) ON DUPLICATE KEY UPDATE SET val1 = %3$d, val2 = %3$d', 'foo-\'bar\'-baz', 'ae4f', NULL );

gives:

INSERT INTO foo ( val1, val2, val3 ) VALUES ( x'ae4f', NULL , 'foo-\'bar\'-baz' ) ON DUPLICATE KEY UPDATE SET val1 = NULL , val2 = NULL

---

Test2:

$wpdb->prepare( 'UPDATE foo SET val1 = %s, val2 = %d, val3 = %f, val4 = %s', 'foo-\'bar\'-baz', NULL, 2013.12, 'Happy Holidays' );

gives:

UPDATE foo SET val1 = 'foo-\'bar\'-baz', val2 = NULL , val3 = 2013.120000, val4 = 'Happy Holidays'

---

Test3:

$wpdb->prepare( 'INSERT INTO foo ( val1, val2, val3, val4 ) VALUES ( %s, %d, %s, %s ) ', 'My First WP post', '1010', NULL, 'Hello, World!' );

gives:

INSERT INTO foo ( val1, val2, val3, val4 ) VALUES ( 'My First WP post', 1010, NULL , 'Hello, World!' )

I see this as one of the possibilities.

[jbutkus]

Patching wpdb::prepare() to support NULL handling

[miqrogroove]

Here are a couple of test cases to break your patch. If you can get the bugs worked out, I'll take another look at it. :)

Patch breaks the last arg:

prepare( 'INSERT INTO foo ( val1, val2, val3, val4, val5 ) VALUES ("\'%%s\' is where strings go",%s,%d,%s,%s) ', 'My First WP post', '1010', NULL, 'Hello, World!' );

Patch returns bool(false) and throws a PHP Warning:

prepare( 'INSERT INTO foo ( val1, val2, val3, val4, val5 ) VALUES ("\'%%s\' is where strings go",%s,%s,%d,%s) ', NULL, 'My First WP post', '1010', 'Hello, World!' );

[miqrogroove]

There is also a case where too few arguments are passed to prepare() and it is supposed to be recognized as an error. You can fix that by checking is_null(), which is currently missing in the patch.

[jbutkus]

Fixed patch for NULLs support in wpdb::prepare()

[jbutkus]

Replying to miqrogroove:

Here are a couple of test cases to break your patch. If you can get the bugs worked out, I'll take another look at it. :)

Thank you for pointing these out. :-)
That was rather trivial to get (solved in regexp), but would have bad effects.

Please see new attachment​ for details.

And maybe you could explain, what would you check via is_null for not being present? To check what vsprintf currently checks for?

[miqrogroove]

Replying to jbutkus:

And maybe you could explain, what would you check via is_null for not being present? To check what vsprintf currently checks for?

I suggest developing some test cases that have arguments missing so that you can see the patch causes prepare() to return a value, which it should not do.

[deltafactory]

While I support the addition of NULL support, I wonder if it would break some plugins that rely on wpdb::prepare() to convert null to zero or an empty string. Would it make sense to support nulls when explicitly enabled, for example:

$wpdb->enable_nulls = true;

[jbutkus]

Fixed patch for NULLs support in wpdb::prepare(), handling arguments count being lower than expected

[jbutkus]

Replying to miqrogroove:

I suggest developing some test cases that have arguments missing so that you can see the patch causes prepare() to return a value, which it should not do.

I would vote for adding it in a different commit, as it is a separate feature (which was not checked for in the past), but here is my attempt upon this​ question.

It sounds logical to resolve it within this very same issue, as now it is easy to add.

[jbutkus]

Replying to deltafactory:

While I support the addition of NULL support, I wonder if it would break some plugins that rely on wpdb::prepare() to convert null to zero or an empty string. Would it make sense to support nulls when explicitly enabled, for example:

$wpdb->enable_nulls = true;

I think that ($wpdb->enable_nulls = true;) would introduce more trouble, than anything else. As somebody would set it, and then forget to reset, while others would expect it to be in one position.

Either it must be another method, that would allow something like this (i.e. create method wpdb::prepare_with_nulls( x, y )), or just attempt to ignore.

How a developer might expect this to be true, when there is no use for NULL values, if wpdb is used regularly, so far?

Albeit it is hard to guess, what a developer might expect.

I, for once, expect wpdb::prepare( x, y ) to work correctly with positional arguments ('INSERT INTO foo SET bar = %2$s, baz = %d'), despite the fact, that it is clearly stated otherwise. And I think there are people who use this, thus I tried to add support for such cases in NULL checking.

So, yes, this is an oldish ticket, which needs some political resolution, I suspect.

[deltafactory]

Replying to jbutkus:

I think that ($wpdb->enable_nulls = true;) would introduce more trouble, than anything else. As somebody would set it, and then forget to reset, while others would expect it to be in one position.

Of course, you're right. My general idea was this: At this point, developers should be required to intentionally choose to use nulls since every bit of code out there was written with the understanding that a null couldn't be inserted. As you help point out, if it's dangerous to enable it within a single page load, it would be disastrous to make the global default.

[miqrogroove]

Replying to jbutkus:

I would vote for adding it in a different commit, as it is a separate feature (which was not checked for in the past), but here is my attempt upon this​ question.

I took a closer look at this for you. I suggest going back to v2.patch and change this line:

if ( ! isset( $values[$loc_index] ) ) {

... with my suggested code ...

if ( $loc_index < count($values) and is_null( $values[$loc_index] ) ) {

... now there is no need for the extra variables.

[miqrogroove]

Disregard previous comment. The call to unset() messes up the array count.

[miqrogroove]

Instead of calling unset(), you could reset $args=array(); at the top of your first if block and then foreach { $args[] = $values[$loc_index] }

Otherwise, $args and $values have the same information being duplicated unnecessarily anyway.

[miqrogroove]

I think a better strategy would be to run two separate loops: foreach($args) to create a list of which args were NULL and unset them, followed by foreach($positions[0]) to modify the query string.

This strategy would avoid duplicating the $args array, which is potentially extremely expensive when working with large queries.

[jbutkus]

More memory-concious approach towards NULL support in wpdb::prepare()

[jbutkus]

Replying to miqrogroove:

I think a better strategy would be to run two separate loops: foreach($args) to create a list of which args were NULL and unset them, followed by foreach($positions[0]) to modify the query string.

This strategy would avoid duplicating the $args array, which is potentially extremely expensive when working with large queries.

Why I propose array_values is that it is faster than foreach by some 65% (several timings running plain foreach with value-copy, vs. array_values, on 10.000 elements array). Thus having two foreach loops shall not give viable performance impact. And we get rid of unnecessary keys.

I have uploaded a new patch​, which may be treated as more memory-concious. Here I got rid of extra $values variable.

What do you think of this approach?

[miqrogroove]

I'm not suggesting a value-copy strategy. With two loops the original $args array can be used.

[arman.poghosyan]

Still no any PDO support in Wordpress and this patch is not in core either.
I tested the patch in different scenarios and while it's working perfectly in INSERT and UPDATE statements, this patch breaks ->prepare for other statements that were working without patch,
for example:

SHOW FULL COLUMNS FROM `table_name`;

I haven't tested it with SELECT statements either. But I still think that we need support for NULL in $wpdb->prepare, as many plugins solely rely on wordpress database functions and they can't operate with NULL values simply because wordpress doesn't support them in prepare, or they must forget about prepare and use other functions directly, than what's the purpose of prepare for security?

[arman.poghosyan]

OK, so other tests showed also that it doesn't work with UPDATE statements either, e.g.

UPDATE `table_name` SET `var`=%s;

is replaced with

UPDATE `table_name` SET `var` NULL ;

it basically strips = sign

[son9ne]

removed.. sorry

[samwilson]

I'm surprised that this ticket has been open for so long. Does anyone have any update on whether it's going to be fixed — or is it just accepted behaviour, and there's too much that relies on it now for it to be changed?

[pento]

15158.2.diff​ is something of a modernisation of the behaviour in 15158.diff​. It's actually a port of ​[backpress 279], which has been running on WordPress.com for years.

Both NULL and 'null' as a case-insensitive string should probably be accepted as a format.

I'm specifically ignoring the format, though I have no problem with documenting that it should be NULL, for clarity of code. @DrewAPicture, can you give your thoughts on updating the docs for ::insert(), ::replace(), ::update() and ::delete()?

With regards to @jbutkus' patches - adding NULL support to ::prepare() is an interesting problem, but it's way outside of the scope of this ticket. Let's handle it in #12819.

[pento]

In 34737:

WPDB: Allow null values in the CRUD functions.

Specifically, ::insert(), ::replace(), ::update(), and ::delete() can now set a column to NULL, or add the IS NULL condition to the WHERE clause.

This is based on ​[backpress 279].

Props pento, nbachiyski, sorich87.

Fixes #15158.

[eveille]

Hello, I noticed that this ticket has been closed some month ago but I recently tried to apply it on woocommerce price to set "no price" instead of 0 through the bulk wordpress interface.
When I type nothing, I still have the "0" value price.

How can I done it ?
Thanks
Samuel

[mikelast]

Hello,
Thanks for implementing this - just for clarification more than anything else is the following now considered correct

<?php
$wpdb->update($ttable, [
                        'user_id' => NULL,
                        'status' => 'available',
                        'update_time' => $now->format('Y-m-d H:i:s')
                ], [
                        'therapist_id' => $therapist_id,
                        'user_id' => $user_id,
                        'start_time' => $ub['start_time']
                ], [
                        NULL,
                        '%s',
                        '%s'
                ], [
                        '%d',
                        '%d',
                        '%s'
                ]);