I have encountered this problem when I try to save big int value by drupal_write_record().
I have defined a field as unsigned big int in the schema, meaning in MySQL I can save value ranging from 0 to 4294967295.
In php, integer value is ranging from -2147483648 to 2147483647.
When I want to save a value like 3453966699, it actually belongs to float type but can be saved as unsigned int(10) not to mention bigint.
But in the drupal_write_record(), all int type including normal and big will be forced to cast to int type.
if ($info['type'] == 'int' || $info['type'] == 'serial') {
$fields[$field] = (int) $fields[$field];
}
So (int) 3453966699 will become -841000597
Can we make it work with unsigned int and big int?
if ($info['type'] == 'int' || $info['type'] == 'serial') {
if ($info['unsigned'] || $info['size'] == 'big') {
$fields[$field] = (float) $fields[$field];
}
else {
$fields[$field] = (int) $fields[$field];
}
}
Note that this primarily affects Windows. For some reason even 64 bit PHP on 64 bit Windows is typecasting int to a 32 bit integer and not 64 - so we are treating it as a string to ensure we are putting in the right number, not a munged float or 2147483647
Comments
Comment #1
ufku commentedI've experienced such issue on Win7(64bit) + PHP(32bit)
A workaround could be to change the field type to 'bigint' in schema definition.
However, there is no solution for very big integers on 32bit PHP. For instance 999999999999999999 will be treated as 1.0E+18 unless it is in string form.
Comment #1.0
ufku commentedAdd example of data type convertion
Comment #2
elusivemind commentedThe problem is bigger than this. Even on 64 bit Windows 10 and 64 bit PHP when you try to insert a bigint value larger than an int, it will only go to 2147483647 ... and value greater than that will stop there.
This is problematic for when I am trying to store values like Twitter tweet id's as a bigint(20) which is defineable by Drupal, but not assignable with drupal_write_record.
Working up a patch now.
Comment #3
elusivemind commentedPatch to correct incorrect typecasting error. Cannot be float as it will change all exponential values not in the number to zeroes.
Comment #4
elusivemind commentedComment #6
elusivemind commentedAdded tests.
Comment #7
elusivemind commentedComment #9
elusivemind commentedComment #10
elusivemind commentedComment #12
elusivemind commentedAddressed testing errors
Comment #13
elusivemind commentedComment #15
elusivemind commentedAddressed test case
Comment #16
elusivemind commentedComment #18
elusivemind commentedComment #19
elusivemind commentedComment #21
elusivemind commentedComment #23
elusivemind commentedComment #25
elusivemind commentedComment #27
elusivemind commentedPerhaps using a reserved word in sql for the test case.
Comment #31
elusivemind commentedComment #32
elusivemind commentedComment #33
elusivemind commentedComment #34
elusivemind commentedWrong test case. Should be asserting against identical not true. Corrected
Comment #36
elusivemind commentedComment #37
elusivemind commentedComment #38
elusivemind commentedComment #40
elusivemind commentedAdded a better test assertion since it is returning the proper value but not testing with the correct test.
Comment #41
elusivemind commented@cilefen -- It looks like this has passed the simpletest. Can I get some more input on this?
Comment #42
elusivemind commentedComment #43
elusivemind commentedComment #44
elusivemind commentedComment #45
cilefen commented@ElusiveMind Could you make a new comment with the existing patch and a test-only patch? Upload the test-only patch first so it will be tested first.
Are we sure this doesn't happen in Drupal 8?
Comment #46
elusivemind commentedI have confirmed that Drupal 8 does not have this issue. I have attached my schema addition and test to show that a bigint is properly handled in Drupal 8. This test passes out of the box. Do we need to include it in the Drupal 8 test suite?
Comment #48
elusivemind commentedthat previous patch was not intended for testing against Drupal 7 ;)
Comment #49
elusivemind commentedComment #50
elusivemind commentedComment #52
elusivemind commentedComment #54
elusivemind commentedIncluding the InsertTest and proper default values for all elements where the schema was changed. Also ensured proper casting and comparisons in test methods.
Comment #55
elusivemind commentedComment #56
elusivemind commentedComment #57
elusivemind commentedOk - in Windows, it is converting to floats - which puts 0000 on the end of the number.
I located the typecasting code in:
https://api.drupal.org/api/drupal/core!includes!schema.inc/function/drup...
But changing that doesn't seem to fix the problem. Still digging, but any help as to where db_insert does it's data typing would be helpful. I've been looking for about 3 hours now.
Comment #58
cilefen commentedComment #59
elusivemind commentedComment #61
elusivemind commentedIssuing the fix patch and test only patch for d7
Comment #64
elusivemind commentedComment #65
elusivemind commentedComment #67
elusivemind commentedThis test passed via the test bot, but in Windows, it fails. Please see the attached screen with the failed test on Windows. Same code.
Comment #68
elusivemind commentedAfter much gnashing of teeth, I think I am going to abandon this and consider drupal_write_record deprecated since it is not in Drupal 8 even though the code from it seems to be there but not doing much.
Using db_insert instead of drupal_write_records seems to resolve the issue present in Windows under Drupal 7. I will document this on the function's page.
Comment #69
elusivemind commentedComment #70
tolstoydotcomI have a custom entity to store tweets. Initially it was just storing the tweet ID as a string, but I updated it to also store the tweet ID as a bigint. On 32-bit Linux, the bigint version of tweet ID was being truncated to 2147483647. That's because my entity extends EntityAPIController, and EntityAPIController::save() uses drupal_write_record(). Applying the patch fixed that.
Since this affects those who call drupal_write_record() directly as well as those who have it called for them, it seems like this is something that would be good to fix on D7.