Opened 7 years ago
Last modified 5 years ago
#43578 new defect (bug)
Unexpected MYSQL data format
Reported by: | loranrendel | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.9.4 |
Component: | Database | Keywords: | 2nd-opinion has-patch has-unit-tests dev-feedback |
Focuses: | Cc: |
Description
When I use field user_id
in $wpdb->insert
it set value to Integer, but the table I add data into has user_id
text field.
It works normally only if format
parameter specified.
Example:
<?php $wpdb->query("CREATE TABLE {$wpdb->prefix}_test (`id` INT, `user_id` VARCHAR(16))"); $wpdb->insert("{$wpdb->prefix}_test", ['id' => 1, 'user_id' => 'stringKey']); print_r($wpdb->get_row("SELECT * FROM {$wpdb->prefix}_test WHERE id = 1"));
Result: stdClass Object ( [id] => 1 [user_id] => 0 )
Attachments (1)
Change History (5)
#1
@
7 years ago
- Keywords needs-patch 2nd-opinion added
- Milestone changed from Awaiting Review to Future Release
#2
@
7 years ago
I think option b) is the greatest one. It doesn't have compatibility issues and would not break anything, because for Wordpress' own tables in this case it will work properly and will get %d
format, and for any other table it will also receive correct format based on table structure.
I think, stop receiving %d
for other tables wouldn't cause back-compat issue because if field is integer it will receive %d
as well, and conversion of string to %d
is incorrect operation.
I encountered this error when converting some old code to WP plugin and got rid of old mysqli wrapper. Some data was lost when added to DB, but I recovered it from logs.
#3
@
7 years ago
- Keywords has-patch has-unit-tests dev-feedback added; needs-patch removed
Interesting case.
43578.diff makes sure that only built-in/core tables abide by the field_types
rules. Custom tables will always default formats to %s
, unless the developer specifically fiddled with $wpdb->field_types
in some way.
How it works, the first time $wpdb->process_field_formats()
is called two internal (static) lists are built - one with the current tables (which is almost certainly going to only contain core tables), and one with the current field types (which is also almost certainly going to only containt core field types).
process_field_formats()
now accepts a third ghost parameter (since it's protected we cannot just add it as a regular parameter since warnings are going to be thrown). A _doing_it_wrong
message is sent out to all subclasses not supplying the third parameter. The third parameter is the $table
.
When the field formats are processed, we make sure that $table
is a built-in table before letting one of the field_types
supply the default format for a field. We also allow custom field overrides to be added to field_types
, these will be respected, except in cases where the field name matches a built-in field.
// without patch "INSERT INTO `wp_test` (`id`, `user_id`) VALUES (%s, %d)" // with patch regular "INSERT INTO `wp_test` (`id`, `user_id`) VALUES (%s, %s)" // with patch custom field $wpdb->field_types['balance'] = '%d'; "INSERT INTO `wp_test` (`id`, `user_id`, `balance`) VALUES (%s, %s, %d)" // with patch custom field collission $wpdb->field_types['user_id'] = '%d'; // not much sense, since it already exists "INSERT INTO `wp_test` (`id`, `user_id`) VALUES (%s, %s)" // note the %s, drawback?
It is probably a bad idea to reformat $wpdb->field_types
, since it is public and can be changed or read by developers. Keeping it as is is crucial. One idea to overcome this limitation is make $field_types
an object that implements ArrayAccess
, and has an add_field_type( $field, $format, $table )
method. This is probably over-engineering, though.
Thoughts?
#4
@
5 years ago
Maybe something like this for non-wordpress tables:
<?php $fields = ($wpdb->get_results("DESC {$wpdb->prefix}regsystem_applications2"));
We will have types for any field
[0] => Array ( [Field] => id [Type] => int(11) [Null] => NO [Key] => PRI [Default] => [Extra] => auto_increment ) [1] => Array ( [Field] => form_id [Type] => int(11) [Null] => YES [Key] => [Default] => [Extra] => ) [2] => Array ( [Field] => record_id [Type] => int(11) [Null] => YES [Key] => [Default] => [Extra] => ) [3] => Array ( [Field] => user_id [Type] => varchar(64) [Null] => YES [Key] => [Default] => [Extra] => )
Hi @loranrendel and welcome back to Trac.
This happens because WordPress has mapped the
user_id
column internally as a numeric field and applies%d
to the values throughinsert()
,replace()
andupdate()
on$wpdb
.For reference, the query generated by the
$wpdb->insert()
call is this:Possible options:
a) Expected behaviour, don't do this, and if you do, specify the data-types
b) Extend the
field_types
to have a table specifier, so that the fields have to be defined on a per-table basis.c) Treat custom tables as custom tables, and only ever use
%s
unless specified otherwise in the call. However$wpdb->field_types
is public and plugins could theoretically update it.#b + #c has a back-compat issue with existing code which is receiving
%d
because it's writing to a custom table with similar column names would start writing with%s
instead, which then uses MySQL's data-type conversion instead.I should say though, that I don't see that as a downside, just a side effect. The benefits would outweigh the downsides.