Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing only some bean properties can fail #547

Closed
noplanman opened this issue Mar 13, 2017 · 12 comments
Closed

Changing only some bean properties can fail #547

noplanman opened this issue Mar 13, 2017 · 12 comments

Comments

@noplanman
Copy link

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 👍

@noplanman
Copy link
Author

@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
Copy link
Owner

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
Copy link
Owner

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

@gabordemooij
Copy link
Owner

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
Copy link
Author

@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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

Can you try this fix? I updated the master.

@noplanman
Copy link
Author

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
Copy link
Owner

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

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

No branches or pull requests

2 participants