Make WordPress Core

Opened 6 years ago

Last modified 5 years ago

#43589 new defect (bug)

WPDB Update change NULL values to acceptable values

Reported by: dorianrd's profile dorianrd Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Database Keywords: has-patch
Focuses: Cc:

Description

Hello,

I'm creating a plugin in which i need to create a new SQL table, that I connect to REST API. My table has several fields that can't be NULL and some that can be NULL.

If i do the update via the wpdb object (cf. below), NULL fields are updated with a NOT NULL value that I've never typed (a float that can't be NULL become 0.00000, a date -0001-00-00, ....).
The expected behaviour would be to not update the line because the request is not OK.

If I allow NULL values on these fields, values are NULL. It's like if somewhere the value was changed to be accepted by the sql engine.

NB: the generated request is OK, when i access to the wpdb->last_query value.


global $wpdb;
$wpdb->update($table, $fields, $where, $fieldsformats, $whereformats);

where $fields = array("mynotnulldate"=> null, "mynotnullfloat" => null);

Dorian

Attachments (1)

43589.diff (4.7 KB) - added by soulseekah 6 years ago.

Download all attachments as: .zip

Change History (6)

#1 @dd32
6 years ago

  • Focuses multisite rest-api removed
  • Keywords reporter-feedback added

Hi @dorianrd,

Could you provide a minimal full set of commands which can be run to duplicate what you're seeing? #43578 is a recent db-related report which includes a great small set of code to duplicate the issue.

Because you say:

NB: the generated request is OK, when i access to the wpdb->last_query value.

it nearly sounds like that this probably isn't a WordPress thing, but is a MySQL behaviour with how you've defined the fields.

#2 @dorianrd
6 years ago

Hi @dd32,

Here are some set of commands to reproduce it :

Table creation

DROP TABLE IF EXISTS `wp_2_wsc_test`;
CREATE TABLE IF NOT EXISTS `wp_2_wsc_test` (
  `title` varchar(250) NOT NULL,
  `address` json NOT NULL,
  `lat` float(10,6) NOT NULL,
  `count` float(10,6) NULL,
 `id` int(11) NOT NULL AUTO_INCREMENT,
  PRIMARY KEY (`id`)
)

PHP commandes

<?php

global $wpdb;
//=>return false. Line not created (> expected behaviour OK)
$ret = $wpdb->insert("wp_2_wsc_test", array('count'=>1, 'title'=>null,'address'=>null,'lat'=>null), array('%d', '%s', '%s', '%f')); 

//=>return 1. Created (expected behaviour OK)   
$ret = $wpdb->insert("wp_2_wsc_test", array('count'=>1, 'title'=>"etet",'address'=>'{"value":"1"}','lat'=>5.5), array('%d', '%s', '%s', '%f'));

//=>return 1. Updated with some values (expected behaviour KO)          
$ret = $wpdb->update("wp_2_wsc_test", array('count'=>1, 'title'=>null,'address'=>null,'lat'=>null), array('id'=>1),  array('%d', '%s', '%s', '%f')); 

These are the values updated:
title => ""
address => "null"
lat => "0.000000"
count => "1"
id => "1"

This is the SQL command generated with $wpdb->last_query :
UPDATE wp_2_wsc_test SET count = '1', title = NULL, address = NULL, lat = NULL WHERE id = '1'

Thus :

  • Insert behaviour is OK
  • Update behaviour is not OK

I don't know if it's a Wordpress or SQL issue, but when I don't use the commands $wpdb->update but the php commands:

<?php
$conn = new mysqli(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME);
$result = $conn->query($req);
$conn->close();

The line is not updated. It seems that the update is caused by the wpdb->update function.

Let me know if you need more information

Have a good day

Dorian

#3 @soulseekah
6 years ago

  • Keywords needs-patch needs-unit-tests added; reporter-feedback removed

There actually seems to be a warning being thrown when trying to insert with a NULL value, or update with a NULL value in the $where parameters.

PHP Notice:  wpdb::prepare was called <strong>incorrectly</strong>. The query argument of wpdb::prepare() must have a placeholder. Please see <a href="https://codex.wordpress.org/Debugging_in_WordPress">Debugging in WordPress</a> for more information. (This message was added in version 3.9.0.) in wp-includes/functions.php on line 4320

This is with columns that allow NULL to be inserted. Otherwise the rest of the behavior seems fine, or at least, yields the correct results.

With columns where NULL is not allowed, inserting errors out as expected:

WordPress database error Column 'title' cannot be null for query INSERT INTO `test2` (`title`) VALUES (NULL)

Updating with a NULL, though, sends in a type-casted value, without throwing a database error. This bug is valid. Reproducible on trunk. While distantly related to #43578, this seems to stem from one of the internal methods in process_fields.

I'd like to work on this. Thanks for the report.

@soulseekah
6 years ago

#4 @soulseekah
6 years ago

  • Keywords needs-patch needs-unit-tests removed

The wpdb::prepare warnings are thrown when inserting a single NULL value.
This happens inside _insert_replace_helper where in this part of the code $this->query( $this->prepare( $sql, $values ) ); the $values is an empty array. For a single NULL value there are no placeholders and no values.

Regarding the update issue. My mistake, this is indeed a MySQL issue. I thought strict mode was assumed by default in WordPress (alas #21212). MySQL does not generate an error, rather generates a warning. I have included tests for this.

UPDATE `test2` SET `title` = NULL WHERE `title` = '2';
Rows matched: 1  Changed: 1  Warnings: 1
SHOW WARNINGS;
+---------+------+-------------------------------+
| Level   | Code | Message                       |
+---------+------+-------------------------------+
| Warning | 1048 | Column 'title' cannot be null |
+---------+------+-------------------------------+

It is, of course, possible to set MySQL into STRICT MODE by issuing a SET sql_mode = 'STRICT_ALL_TABLES'; query (https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict) in order to force it to produce errors where warnings are produced by default. But be careful with this. A lot in core is supposed to break under strict mode, and definitely third-party plugins and themes will break.

Tests for this are also present in 43989.diff. While this report is invalid, it did help uncover two warnings from $wpdb->prepare and added some new tests for redundancy.

#5 @pento
5 years ago

  • Keywords has-patch added
  • Version trunk deleted
Note: See TracTickets for help on using tickets.