Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixing bug. RedBean will correctly store an existing bean when... #132

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

thaya commented Feb 22, 2012

... you do R::store($bean), if the bean's id is a BIGINT. This is because the query writer type casts the id string to an int before it adds it to the UPDATE statement.

I'm not sure if this is the right way to fix this or not, but it works for me. Here's the repro code:

$bean = R::load('bean', $bean_id);
$bean->some_other_column = 1234;
R::store($bean);

R::store doesn't store any information if the bean's id is a BIGINT (e.g. something like 113989058729911) because the id gets type casted to an int and resolves to the INTMAX value.

@thaya thaya Fixing bug. RedBean will not store correctly store an existing bean w…
…hen you do R::store($bean), if the bean's id is a BIGINT. This is because the query writer type casts the id string to an int before it adds it to the UPDATE statement.
7d18f44
Contributor

damianb commented Feb 22, 2012

Storing the ID a string wouldn't be the best of ideas IMO - both php's floats and mysql's bigints seem to be equivalent in range on a little research, so I'm of the opinion that (float) casting would be a better option instead.

http://php.net/manual/en/language.types.float.php
http://dev.mysql.com/doc/refman/5.0/en/numeric-type-overview.html

thaya commented Feb 22, 2012

Thanks for the quick research and response @damianb.

I see your point that storing the ID as a string wouldn't be the best of ideas. Good thing is that we are not storing the ID as strings in our database, it is still BIGINT.

The code change was made so that the UPDATE statement can be built properly. I haven't had much experience with machine-dependent precision/rounding issues when casting things to (float), I wonder if something like the following can become a problem when we do " WHERE id = ".(float) $id?

http://www.mysqlperformanceblog.com/2008/01/10/php-vs-bigint-vs-float-conversion-caveat/.

Contributor

damianb commented Feb 22, 2012

Hmm. Good point - though it should be noted that the PHP4 results aren't too reliable as there may have been fixes since then. Also caught that update note at the bottom:

UPDATE: as Jakub Vrana pointed out in the comments (thanks!), it’s the “precision” option set from php.ini which affects the conversion. I’ve played with it a bit; the compiled-in and php.ini-dist values seem to vary across architectures and versions, but setting precision=16 (enough to hold 2^53 in decimal without sign) helped in all cases.

That may be something that should be pointed out as a fix if using float typecast. At any stage, though, we're all SOL when it comes to beyond 2^53 range it seems unless we do some sort of string magic, which if there's any parameter binding going on internally in redbean (i'm not sure how that works) it could blow up in short order.

That and if the ID was ever changed manually, things would go wrong very quickly - but that something that shouldn't be necessary in an ORM anyways (unless someone has a legitimate use case for that that'd like to speak up - I can't think of anything for that).

Owner

gabordemooij commented Mar 21, 2012

Actually if you use PHP on 64bit architecture it uses 8 bytes for int which is the max for BIGINT as well.
So the current solution works fine on 64bit architectures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment