gabordemooij / redbean

ORM layer that creates models, config and database on the fly
https://www.redbeanphp.com
2.31k stars 279 forks source link

Changing only some bean properties can fail #547

Closed noplanman closed 7 years ago

noplanman commented 7 years ago

When I load a bean (via $bean = R::findOne(...);), only change a few values and then call R::store($bean);, it can happen that I get an error like: Error in SQL query: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for type boolean: ""

This happens, when there is a boolean field set to false and I don't update that field. When R::store() is called, the value isn't translated to '0' but instead stays false. PostgreSQL doesn't like this.

When setting a boolean value, it gets converted correctly, but any unchanged values stay of type boolean.

This can be fixed by (re)setting all properties, like so, in RedBeanPHP/Repository/Frozen.php just before getting the properties to be saved.

foreach( $bean->getProperties() as $key => $value ) {
  $bean[$key] = $value;
}
list( $properties, $table ) = $bean->getPropertiesAndType();
...

Surely not the cleanest way, but it works.

Alternatively, I could run that foreach loop just before calling R::store($bean);, but I think this should be part of the library itself.

Hope this is explained well enough, else just ask :+1:

noplanman commented 7 years ago

@gabordemooij Any input on this?

At the moment I have to run the foreach loop before every R::store() which is a bit nasty 😁

gabordemooij commented 7 years ago

Hi there, thank you for reporting this. I have been really busy these months so I did not have time to answer all requests properly. However that's going to change, so I will look into this.

gabordemooij commented 7 years ago

I am setting up a new RedBeanPHP devbox as we speak...

gabordemooij commented 7 years ago

Hm I tried to reproduce this issue, but I couldnt...


$bean = R::dispense('bean');
$bean->yes = false;
$bean->bla = 'aaa';
$id = R::store($bean);

R::freeze(true);
$bean = R::load('bean', $id);
$bean->bla = 'bbb';
R::store( $bean );
print_r(R::load('bean', $id));

this works just fine... did you add the boolean type column by yourself?

noplanman commented 7 years ago

@gabordemooij Thanks so much for looking into this!

Aah, I see, because your column is set as an integer for the boolean by default. What I have a is an existing table with the boolean columns set to boolean data type.

Try this and then retest your exact code:

ALTER TABLE bean ALTER COLUMN yes TYPE boolean USING yes::boolean;
gabordemooij commented 7 years ago

Okay, yes this probably why RedBeanPHP fails, I can now reproduce the issue. Easiest way to solve this would be to create a cast for text to boolean, I think I'll research that option first. I also liked your idea about only storing partial beans (only changed properties) but then I remembered people seem to depend on this behavior (i.e. race-conditions, duplications etc).

gabordemooij commented 7 years ago

For some reason, I dont seem to able to create a cast in PostgreSQL to convert a value of 'type unknown' which seems to be the case for literals to 'boolean'. I am now working on a solution using the partial-save you proposed. I think I'll add an option to activate this behavior so it remains backward compatible: R::usePartialBeans( true ); what do you think?

noplanman commented 7 years ago

Hi @gabordemooij Thanks again for looking into this.

Absolutely @ remains backward compatible. I think R::usePartialBeans( true ); sounds great. Setting this option would then basically only update the properties that have been changed, right?

As far as I saw when browsing the code, each property gets a meta field assigned to it that remembers if it's been changed or not, right?

gabordemooij commented 7 years ago

Not really, only if 'a property' has been changed. To this end I added the meta property 'changelist'. The changelist will keep track of changed properties.

gabordemooij commented 7 years ago

Can you try this fix? I updated the master.

noplanman commented 7 years ago

me trying without adding R::usePartialBeans(true); Hmm, still doesn't work...

me realising that I forgot R::usePartialBeans(true); then add it Woohoo, it's working perfectly! 🎉

Looking forward to the next release! 👍

gabordemooij commented 7 years ago

Thanks for confirming the fix. I'll release a new version asap.